fix: Update RegisterCommands to return error in start.go

cmd/cli/commands/start/start.go

@@ -3,81 +3,80 @@ package start
 import (
 	"fmt"
 
-	"github.com/urfave/cli"
-
 	"github.com/ethpandaops/contributoor-installer/cmd/cli/options"
 	"github.com/ethpandaops/contributoor-installer/internal/service"
 	"github.com/ethpandaops/contributoor-installer/internal/tui"
+	"github.com/sirupsen/logrus"
+	"github.com/urfave/cli"
 )
 
-func RegisterCommands(app *cli.App, opts *options.CommandOpts) {
+func RegisterCommands(app *cli.App, opts *options.CommandOpts) error {
 	app.Commands = append(app.Commands, cli.Command{
 		Name:      opts.Name(),
 		Aliases:   opts.Aliases(),
 		Usage:     "Start Contributoor",
 		UsageText: "contributoor start [options]",
 		Action: func(c *cli.Context) error {
-			return startContributoor(c, opts)
+			log := opts.Logger()
+
+			configService, err := service.NewConfigService(log, c.GlobalString("config-path"))
+			if err != nil {
+				return fmt.Errorf("error loading config: %w", err)
+			}
+
+			dockerService, err := service.NewDockerService(log, configService)
+			if err != nil {
+				return fmt.Errorf("error creating docker service: %w", err)
+			}
+
+			binaryService := service.NewBinaryService(log, configService)
+
+			return startContributoor(c, log, configService, dockerService, binaryService)
 		},
 	})
+
+	return nil
 }
 
-func startContributoor(c *cli.Context, opts *options.CommandOpts) error {
-	log := opts.Logger()
+func startContributoor(
+	c *cli.Context,
+	log *logrus.Logger,
+	config service.ConfigManager,
+	docker service.DockerService,
+	binary service.BinaryService,
+) error {
+	var (
+		runner service.ServiceRunner
+		cfg    = config.Get()
+	)
 
-	configService, err := service.NewConfigService(log, c.GlobalString("config-path"))
-	if err != nil {
-		return fmt.Errorf("%sError loading config: %v%s", tui.TerminalColorRed, err, tui.TerminalColorReset)
-	}
+	log.WithField("version", cfg.Version).Info("Starting Contributoor")
 
 	// Start the service via whatever method the user has configured (docker or binary).
-	switch configService.Get().RunMethod {
+	switch cfg.RunMethod {
 	case service.RunMethodDocker:
-		log.WithField("version", configService.Get().Version).Info("Starting Contributoor")
-
-		dockerService, err := service.NewDockerService(log, configService)
-		if err != nil {
-			log.Errorf("could not create docker service: %v", err)
-
-			return err
-		}
-
-		// Check if the service is already running.
-		running, err := dockerService.IsRunning()
-		if err != nil {
-			log.Errorf("could not check service status: %v", err)
-
-			return err
-		}
+		runner = docker
+	case service.RunMethodBinary:
+		runner = binary
+	default:
+		return fmt.Errorf("invalid run method: %s", cfg.RunMethod)
+	}
 
-		// If the service is already running, we can just return.
-		if running {
-			return fmt.Errorf("%sContributoor is already running. Use 'contributoor stop' first if you want to restart it%s", tui.TerminalColorRed, tui.TerminalColorReset)
-		}
+	// Check if the service is already running.
+	running, err := runner.IsRunning()
+	if err != nil {
+		log.Errorf("could not check service status: %v", err)
 
-		if err := dockerService.Start(); err != nil {
-			log.Errorf("could not start service: %v", err)
+		return err
+	}
 
-			return err
-		}
+	// If the service is already running, we can just return.
+	if running {
+		return fmt.Errorf("%sContributoor is already running. Use 'contributoor stop' first if you want to restart it%s", tui.TerminalColorRed, tui.TerminalColorReset)
+	}
 
-	case service.RunMethodBinary:
-		binaryService := service.NewBinaryService(log, configService)
-
-		// Check if the service is currently running.
-		running, err := binaryService.IsRunning()
-		if err != nil {
-			return fmt.Errorf("failed to check service status: %v", err)
-		}
-
-		// If the service is already running, we can just return.
-		if running {
-			return fmt.Errorf("%sContributoor is already running%s", tui.TerminalColorRed, tui.TerminalColorReset)
-		}
-
-		if err := binaryService.Start(); err != nil {
-			return err
-		}
+	if err := runner.Start(); err != nil {
+		return err
 	}
 
 	return nil

cmd/cli/commands/start/start_test.go

@@ -0,0 +1,204 @@
+package start
+
+import (
+	"errors"
+	"flag"
+	"testing"
+
+	"github.com/ethpandaops/contributoor-installer/cmd/cli/options"
+	"github.com/ethpandaops/contributoor-installer/internal/service"
+	"github.com/ethpandaops/contributoor-installer/internal/service/mock"
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli"
+	"go.uber.org/mock/gomock"
+)
+
+func TestStartContributoor(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	tests := []struct {
+		name          string
+		runMethod     string
+		setupMocks    func(*mock.MockConfigManager, *mock.MockDockerService, *mock.MockBinaryService)
+		expectedError string
+	}{
+		{
+			name:      "docker - starts service successfully",
+			runMethod: service.RunMethodDocker,
+			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerService, b *mock.MockBinaryService) {
+				cfg.EXPECT().Get().Return(&service.ContributoorConfig{
+					RunMethod: service.RunMethodDocker,
+					Version:   "latest",
+				}).Times(1)
+				d.EXPECT().IsRunning().Return(false, nil)
+				d.EXPECT().Start().Return(nil)
+			},
+		},
+		{
+			name:      "docker - service already running",
+			runMethod: service.RunMethodDocker,
+			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerService, b *mock.MockBinaryService) {
+				cfg.EXPECT().Get().Return(&service.ContributoorConfig{
+					RunMethod: service.RunMethodDocker,
+				}).Times(1)
+				d.EXPECT().IsRunning().Return(true, nil)
+			},
+			expectedError: "Contributoor is already running",
+		},
+		{
+			name:      "docker - start fails",
+			runMethod: service.RunMethodDocker,
+			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerService, b *mock.MockBinaryService) {
+				cfg.EXPECT().Get().Return(&service.ContributoorConfig{
+					RunMethod: service.RunMethodDocker,
+				}).Times(1)
+				d.EXPECT().IsRunning().Return(false, nil)
+				d.EXPECT().Start().Return(errors.New("start failed"))
+			},
+			expectedError: "start failed",
+		},
+		{
+			name:      "binary - starts service successfully",
+			runMethod: service.RunMethodBinary,
+			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerService, b *mock.MockBinaryService) {
+				cfg.EXPECT().Get().Return(&service.ContributoorConfig{
+					RunMethod: service.RunMethodBinary,
+				}).Times(1)
+				b.EXPECT().IsRunning().Return(false, nil)
+				b.EXPECT().Start().Return(nil)
+			},
+		},
+		{
+			name:      "binary - service already running",
+			runMethod: service.RunMethodBinary,
+			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerService, b *mock.MockBinaryService) {
+				cfg.EXPECT().Get().Return(&service.ContributoorConfig{
+					RunMethod: service.RunMethodBinary,
+				}).Times(1)
+				b.EXPECT().IsRunning().Return(true, nil)
+			},
+			expectedError: "Contributoor is already running",
+		},
+		{
+			name:      "invalid run method",
+			runMethod: "invalid",
+			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerService, b *mock.MockBinaryService) {
+				cfg.EXPECT().Get().Return(&service.ContributoorConfig{
+					RunMethod: "invalid",
+				}).Times(1)
+			},
+			expectedError: "invalid run method",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockConfig := mock.NewMockConfigManager(ctrl)
+			mockDocker := mock.NewMockDockerService(ctrl)
+			mockBinary := mock.NewMockBinaryService(ctrl)
+
+			tt.setupMocks(mockConfig, mockDocker, mockBinary)
+
+			app := cli.NewApp()
+			ctx := cli.NewContext(app, nil, nil)
+
+			err := startContributoor(ctx, logrus.New(), mockConfig, mockDocker, mockBinary)
+
+			if tt.expectedError != "" {
+				assert.ErrorContains(t, err, tt.expectedError)
+
+				return
+			}
+
+			assert.NoError(t, err)
+		})
+	}
+}
+
+func TestRegisterCommands(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	tests := []struct {
+		name          string
+		configPath    string
+		expectedError string
+	}{
+		{
+			name:       "successfully registers command",
+			configPath: "testdata/valid", // "testdata" is an ancillary dir provided by go-test.
+		},
+		{
+			name:          "fails when config service fails",
+			configPath:    "/invalid/path/that/doesnt/exist",
+			expectedError: "error loading config",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create CLI app, with the config flag.
+			app := cli.NewApp()
+			app.Flags = []cli.Flag{
+				cli.StringFlag{
+					Name: "config-path",
+				},
+			}
+
+			// Ensure we set the config path flag.
+			globalSet := flag.NewFlagSet("test", flag.ContinueOnError)
+			globalSet.String("config-path", "", "")
+			err := globalSet.Set("config-path", tt.configPath)
+			require.NoError(t, err)
+
+			// Create the cmd context.
+			globalCtx := cli.NewContext(app, globalSet, nil)
+			app.Metadata = map[string]interface{}{
+				"flagContext": globalCtx,
+			}
+
+			// Now test!
+			err = RegisterCommands(
+				app,
+				options.NewCommandOpts(
+					options.WithName("start"),
+					options.WithLogger(logrus.New()),
+					options.WithAliases([]string{"s"}),
+				),
+			)
+
+			if tt.expectedError != "" {
+				// Ensure the command registration succeeded.
+				assert.NoError(t, err)
+
+				// Assert that the action execution fails as expected.
+				cmd := app.Commands[0]
+				ctx := cli.NewContext(app, nil, globalCtx)
+
+				// Assert that the action is the func we expect, mainly because the linter is having a fit otherwise.
+				action, ok := cmd.Action.(func(*cli.Context) error)
+				require.True(t, ok, "expected action to be func(*cli.Context) error")
+
+				// Execute the action and assert the error.
+				actionErr := action(ctx)
+				assert.Error(t, actionErr)
+				assert.ErrorContains(t, actionErr, tt.expectedError)
+			} else {
+				// Ensure the command registration succeeded.
+				assert.NoError(t, err)
+				assert.Len(t, app.Commands, 1)
+
+				// Ensure the command is registered as expected by dumping the command.
+				cmd := app.Commands[0]
+				assert.Equal(t, "start", cmd.Name)
+				assert.Equal(t, []string{"s"}, cmd.Aliases)
+				assert.Equal(t, "Start Contributoor", cmd.Usage)
+				assert.Equal(t, "contributoor start [options]", cmd.UsageText)
+				assert.NotNil(t, cmd.Action)
+			}
+		})
+	}
+}

cmd/cli/main.go

@@ -63,10 +63,12 @@ func main() {
 		options.WithAliases([]string{"i"}),
 	))
 
-	start.RegisterCommands(app, options.NewCommandOpts(
+	if err := start.RegisterCommands(app, options.NewCommandOpts(
 		options.WithName("start"),
 		options.WithLogger(log),
-	))
+	)); err != nil {
+		log.Errorf("failed to register start command: %v", err)
+	}
 
 	if err := stop.RegisterCommands(app, options.NewCommandOpts(
 		options.WithName("stop"),