feat(plugins): Wire the test verb up to the CLI plugin infrastructure (…

…#91)

cmd/aspect/root/root.go

@@ -83,7 +83,7 @@ func NewRootCmd(
 	cmd.AddCommand(version.NewDefaultVersionCmd())
 	cmd.AddCommand(docs.NewDefaultDocsCmd())
 	cmd.AddCommand(info.NewDefaultInfoCmd())
-	cmd.AddCommand(test.NewDefaultTestCmd())
+	cmd.AddCommand(test.NewDefaultTestCmd(pluginSystem))
 
 	// ### "Additional help topic commands" which are not runnable
 	// https://pkg.go.dev/github.com/spf13/cobra#Command.IsAdditionalHelpTopicCommand

cmd/aspect/test/BUILD.bazel

@@ -6,10 +6,14 @@ go_library(
     importpath = "aspect.build/cli/cmd/aspect/test",
     visibility = ["//visibility:public"],
     deps = [
+        "//cmd/aspect/root/flags",
         "//pkg/aspect/test",
+        "//pkg/aspecterrors",
         "//pkg/bazel",
         "//pkg/interceptors",
         "//pkg/ioutils",
+        "//pkg/plugin/system",
+        "//pkg/plugin/system/bep",
         "@com_github_spf13_cobra//:cobra",
     ],
 )

cmd/aspect/test/test.go

@@ -8,20 +8,36 @@ package test
 
 import (
 	"context"
+	"errors"
+	"fmt"
 
 	"github.com/spf13/cobra"
 
+	rootFlags "aspect.build/cli/cmd/aspect/root/flags"
 	"aspect.build/cli/pkg/aspect/test"
+	"aspect.build/cli/pkg/aspecterrors"
 	"aspect.build/cli/pkg/bazel"
 	"aspect.build/cli/pkg/interceptors"
 	"aspect.build/cli/pkg/ioutils"
+	"aspect.build/cli/pkg/plugin/system"
+	"aspect.build/cli/pkg/plugin/system/bep"
 )
 
-func NewDefaultTestCmd() *cobra.Command {
-	return NewTestCmd(ioutils.DefaultStreams, bazel.New())
+// NewDefaultTestCmd creates a new build cobra command with the default
+// dependencies.
+func NewDefaultTestCmd(pluginSystem system.PluginSystem) *cobra.Command {
+	return NewTestCmd(
+		ioutils.DefaultStreams,
+		pluginSystem,
+		bazel.New(),
+	)
 }
 
-func NewTestCmd(streams ioutils.Streams, bzl bazel.Bazel) *cobra.Command {
+func NewTestCmd(
+	streams ioutils.Streams,
+	pluginSystem system.PluginSystem,
+	bzl bazel.Bazel,
+) *cobra.Command {
 	return &cobra.Command{
 		Use:   "test",
 		Short: "Builds the specified targets and runs all test targets among them.",
@@ -39,12 +55,32 @@ specify targets.
 		RunE: interceptors.Run(
 			[]interceptors.Interceptor{
 				interceptors.WorkspaceRootInterceptor(),
+				pluginSystem.BESBackendInterceptor(),
 			},
 			func(ctx context.Context, cmd *cobra.Command, args []string) (exitErr error) {
+				isInteractiveMode, err := cmd.Root().PersistentFlags().GetBool(rootFlags.InteractiveFlagName)
+				if err != nil {
+					return err
+				}
+
+				defer func() {
+					errs := pluginSystem.ExecutePostTest(isInteractiveMode).Errors()
+					if len(errs) > 0 {
+						for _, err := range errs {
+							fmt.Fprintf(streams.Stderr, "Error: failed to run build command: %v\n", err)
+						}
+						var err *aspecterrors.ExitError
+						if errors.As(exitErr, &err) {
+							err.ExitCode = 1
+						}
+					}
+				}()
+
 				workspaceRoot := ctx.Value(interceptors.WorkspaceRootKey).(string)
 				bzl.SetWorkspaceRoot(workspaceRoot)
 				t := test.New(streams, bzl)
-				return t.Run(cmd, args)
+				besBackend := ctx.Value(system.BESBackendInterceptorKey).(bep.BESBackend)
+				return t.Run(args, besBackend)
 			},
 		),
 	}

pkg/aspect/test/BUILD.bazel

@@ -9,6 +9,7 @@ go_library(
         "//pkg/aspecterrors",
         "//pkg/bazel",
         "//pkg/ioutils",
+        "//pkg/plugin/system/bep",
         "@com_github_spf13_cobra//:cobra",
     ],
 )
@@ -20,6 +21,7 @@ go_test(
         ":test",
         "//pkg/bazel/mock",
         "//pkg/ioutils",
+        "//pkg/plugin/system/bep/mock",
         "@com_github_golang_mock//gomock",
         "@com_github_onsi_gomega//:gomega",
     ],

pkg/aspect/test/test.go

@@ -7,11 +7,12 @@ Not licensed for re-use.
 package test
 
 import (
-	"aspect.build/cli/pkg/bazel"
-	"aspect.build/cli/pkg/ioutils"
-	"github.com/spf13/cobra"
+	"fmt"
 
 	"aspect.build/cli/pkg/aspecterrors"
+	"aspect.build/cli/pkg/bazel"
+	"aspect.build/cli/pkg/ioutils"
+	"aspect.build/cli/pkg/plugin/system/bep"
 )
 
 type Test struct {
@@ -26,14 +27,26 @@ func New(streams ioutils.Streams, bzl bazel.Bazel) *Test {
 	}
 }
 
-func (t *Test) Run(_ *cobra.Command, args []string) error {
-	bazelCmd := []string{"test"}
+func (t *Test) Run(args []string, besBackend bep.BESBackend) (exitErr error) {
+	besBackendFlag := fmt.Sprintf("--bes_backend=grpc://%s", besBackend.Addr())
+	bazelCmd := []string{"test", besBackendFlag}
 	bazelCmd = append(bazelCmd, args...)
 
-	if exitCode, err := t.bzl.Spawn(bazelCmd); exitCode != 0 {
-		err = &aspecterrors.ExitError{
-			Err:      err,
-			ExitCode: exitCode,
+	exitCode, bazelErr := t.bzl.Spawn(bazelCmd)
+
+	// Process the subscribers errors before the Bazel one.
+	subscriberErrors := besBackend.Errors()
+	if len(subscriberErrors) > 0 {
+		for _, err := range subscriberErrors {
+			fmt.Fprintf(t.Streams.Stderr, "Error: failed to run test command: %v\n", err)
+		}
+		exitCode = 1
+	}
+
+	if exitCode != 0 {
+		err := &aspecterrors.ExitError{ExitCode: exitCode}
+		if bazelErr != nil {
+			err.Err = bazelErr
 		}
 		return err
 	}

pkg/aspect/test/test_test.go

@@ -3,11 +3,13 @@ package test_test
 import (
 	"testing"
 
+	"github.com/golang/mock/gomock"
+	. "github.com/onsi/gomega"
+
 	"aspect.build/cli/pkg/aspect/test"
 	"aspect.build/cli/pkg/bazel/mock"
 	"aspect.build/cli/pkg/ioutils"
-	"github.com/golang/mock/gomock"
-	. "github.com/onsi/gomega"
+	bep_mock "aspect.build/cli/pkg/plugin/system/bep/mock"
 )
 
 // Embrace the stutter :)
@@ -20,10 +22,21 @@ func TestTest(t *testing.T) {
 		bzl := mock.NewMockBazel(ctrl)
 		bzl.
 			EXPECT().
-			Spawn([]string{"test"}).
+			Spawn([]string{"test", "--bes_backend=grpc://127.0.0.1:12345"}).
 			Return(0, nil)
 
+		besBackend := bep_mock.NewMockBESBackend(ctrl)
+		besBackend.
+			EXPECT().
+			Addr().
+			Return("127.0.0.1:12345").
+			Times(1)
+		besBackend.
+			EXPECT().
+			Errors().
+			Times(1)
+
 		b := test.New(ioutils.Streams{}, bzl)
-		g.Expect(b.Run(nil, []string{})).Should(Succeed())
+		g.Expect(b.Run([]string{}, besBackend)).Should(Succeed())
 	})
 }

pkg/plugin/sdk/v1alpha2/plugin/grpc.go

@@ -74,6 +74,25 @@ func (m *GRPCServer) PostBuildHook(
 		m.Impl.PostBuildHook(req.IsInteractiveMode, prompter)
 }
 
+// PostTestHook translates the gRPC call to the Plugin PostTestHook
+// implementation. It starts a prompt runner that is passed to the Plugin
+// instance to be able to perform prompt actions to the CLI user.
+func (m *GRPCServer) PostTestHook(
+	ctx context.Context,
+	req *proto.PostTestHookReq,
+) (*proto.PostTestHookRes, error) {
+	conn, err := m.broker.Dial(req.BrokerId)
+	if err != nil {
+		return nil, err
+	}
+	defer conn.Close()
+
+	client := proto.NewPrompterClient(conn)
+	prompter := &PrompterGRPCClient{client: client}
+	return &proto.PostTestHookRes{},
+		m.Impl.PostTestHook(req.IsInteractiveMode, prompter)
+}
+
 // GRPCClient implements the gRPC client that is used by the Core to communicate
 // with the Plugin instances.
 type GRPCClient struct {
@@ -109,6 +128,27 @@ func (m *GRPCClient) PostBuildHook(isInteractiveMode bool, promptRunner ioutils.
 	return err
 }
 
+// PostTestHook is called from the Core to execute the Plugin PostTestHook. It
+// starts the prompt runner server with the provided PromptRunner.
+func (m *GRPCClient) PostTestHook(isInteractiveMode bool, promptRunner ioutils.PromptRunner) error {
+	prompterServer := &PrompterGRPCServer{promptRunner: promptRunner}
+	var s *grpc.Server
+	serverFunc := func(opts []grpc.ServerOption) *grpc.Server {
+		s = grpc.NewServer(opts...)
+		proto.RegisterPrompterServer(s, prompterServer)
+		return s
+	}
+	brokerID := m.broker.NextId()
+	go m.broker.AcceptAndServe(brokerID, serverFunc)
+	req := &proto.PostTestHookReq{
+		BrokerId:          brokerID,
+		IsInteractiveMode: isInteractiveMode,
+	}
+	_, err := m.client.PostTestHook(context.Background(), req)
+	s.Stop()
+	return err
+}
+
 // PrompterGRPCServer implements the gRPC server that runs on the Core and is
 // passed to the Plugin to allow prompt actions to the CLI user.
 type PrompterGRPCServer struct {

pkg/plugin/sdk/v1alpha2/plugin/interface.go

@@ -18,4 +18,8 @@ type Plugin interface {
 		isInteractiveMode bool,
 		promptRunner ioutils.PromptRunner,
 	) error
+	PostTestHook(
+		isInteractiveMode bool,
+		promptRunner ioutils.PromptRunner,
+	) error
 }

pkg/plugin/sdk/v1alpha2/proto/plugin.proto

@@ -10,6 +10,7 @@ option go_package = "aspect.build/cli/pkg/plugin/sdk/v1alpha2/proto";
 service Plugin {
   rpc BEPEventCallback(BEPEventCallbackReq) returns (BEPEventCallbackRes);
   rpc PostBuildHook(PostBuildHookReq) returns (PostBuildHookRes);
+  rpc PostTestHook(PostTestHookReq) returns (PostTestHookRes);
 }
 
 message BEPEventCallbackReq {
@@ -25,6 +26,13 @@ message PostBuildHookReq {
 
 message PostBuildHookRes {}
 
+message PostTestHookReq {
+  uint32 broker_id = 1;
+  bool is_interactive_mode = 2;
+}
+
+message PostTestHookRes {}
+
 // Prompter is the service used by the Plugin instances to request prompt
 // actions to the Core from the CLI users.
 service Prompter {

pkg/plugin/system/BUILD.bazel

@@ -12,8 +12,8 @@ go_library(
         "//pkg/aspecterrors",
         "//pkg/interceptors",
         "//pkg/ioutils",
-        "//pkg/plugin/sdk/v1alpha1/config",
-        "//pkg/plugin/sdk/v1alpha1/plugin",
+        "//pkg/plugin/sdk/v1alpha2/config",
+        "//pkg/plugin/sdk/v1alpha2/plugin",
         "//pkg/plugin/system/bep",
         "@com_github_hashicorp_go_hclog//:go-hclog",
         "@com_github_hashicorp_go_plugin//:go-plugin",

pkg/plugin/system/README.md

@@ -38,4 +38,4 @@ on which hooks are exposed.
 
 ## Current SDK
 
-See [the current SDK README](/pkg/plugin/sdk/v1alpha1/README.md).
+See [the current SDK README](/pkg/plugin/sdk/v1alpha2/README.md).

pkg/plugin/system/system.go

@@ -19,8 +19,8 @@ import (
 	"aspect.build/cli/pkg/aspecterrors"
 	"aspect.build/cli/pkg/interceptors"
 	"aspect.build/cli/pkg/ioutils"
-	"aspect.build/cli/pkg/plugin/sdk/v1alpha1/config"
-	"aspect.build/cli/pkg/plugin/sdk/v1alpha1/plugin"
+	"aspect.build/cli/pkg/plugin/sdk/v1alpha2/config"
+	"aspect.build/cli/pkg/plugin/sdk/v1alpha2/plugin"
 	"aspect.build/cli/pkg/plugin/system/bep"
 )
 
@@ -31,6 +31,7 @@ type PluginSystem interface {
 	TearDown()
 	BESBackendInterceptor() interceptors.Interceptor
 	ExecutePostBuild(isInteractiveMode bool) *aspecterrors.ErrorList
+	ExecutePostTest(isInteractiveMode bool) *aspecterrors.ErrorList
 }
 
 type pluginSystem struct {
@@ -155,6 +156,17 @@ func (ps *pluginSystem) ExecutePostBuild(isInteractiveMode bool) *aspecterrors.E
 	return errors
 }
 
+// ExecutePostTest executes all post-build hooks from all plugins.
+func (ps *pluginSystem) ExecutePostTest(isInteractiveMode bool) *aspecterrors.ErrorList {
+	errors := &aspecterrors.ErrorList{}
+	for node := ps.plugins.head; node != nil; node = node.next {
+		if err := node.plugin.PostTestHook(isInteractiveMode, ps.promptRunner); err != nil {
+			errors.Insert(err)
+		}
+	}
+	return errors
+}
+
 // ClientFactory hides the call to goplugin.NewClient.
 type ClientFactory interface {
 	New(*goplugin.ClientConfig) ClientProvider

plugins/fix-visibility/BUILD.bazel

@@ -8,7 +8,7 @@ go_library(
     deps = [
         "//bazel/buildeventstream/proto",
         "//pkg/ioutils",
-        "//pkg/plugin/sdk/v1alpha1/config",
+        "//pkg/plugin/sdk/v1alpha2/config",
         "@bazel_gazelle//label:go_default_library",
         "@com_github_bazelbuild_buildtools//edit:go_default_library",
         "@com_github_hashicorp_go_plugin//:go-plugin",

plugins/fix-visibility/plugin.go

@@ -26,7 +26,7 @@ import (
 
 	buildeventstream "aspect.build/cli/bazel/buildeventstream/proto"
 	"aspect.build/cli/pkg/ioutils"
-	"aspect.build/cli/pkg/plugin/sdk/v1alpha1/config"
+	"aspect.build/cli/pkg/plugin/sdk/v1alpha2/config"
 )
 
 func main() {
@@ -154,6 +154,17 @@ func (plugin *FixVisibilityPlugin) PostBuildHook(
 	return nil
 }
 
+// PostTestHook satisfies the Plugin interface. It prompts the user for
+// automatic fixes when in interactive mode. If the user rejects the automatic
+// fixes, or if running in non-interactive mode, the commands to perform the fixes
+// are printed to the terminal.
+func (plugin *FixVisibilityPlugin) PostTestHook(
+	isInteractiveMode bool,
+	promptRunner ioutils.PromptRunner,
+) error {
+	return plugin.PostBuildHook(isInteractiveMode, promptRunner)
+}
+
 func (plugin *FixVisibilityPlugin) hasPrivateVisibility(toFix string) (bool, error) {
 	visibility, err := plugin.buildozer.run("print visibility", toFix)
 	if err != nil {