(feat): move from exit to return and Run to RunE to make things testable

cmd/cleanup.go

@@ -16,6 +16,7 @@ limitations under the License.
 package cmd
 
 import (
+	"fmt"
 	"log/slog"
 	"os"
 
@@ -28,7 +29,7 @@ import (
 var cleanupCmd = &cobra.Command{
 	Use:   "cleanup",
 	Short: "cleanup removes all services with the tag prefix from a given consul service",
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{
 			Level: slog.LevelInfo,
 		}))
@@ -40,7 +41,7 @@ var cleanupCmd = &cobra.Command{
 		consulClient, err := api.NewClient(config)
 		if err != nil {
 			logger.Error("Failed to create Consul client", "error", err)
-			os.Exit(1)
+			return fmt.Errorf("failed to create Consul client: %w", err)
 		}
 
 		serviceID := cmd.InheritedFlags().Lookup("service-id").Value.String()
@@ -61,10 +62,11 @@ var cleanupCmd = &cobra.Command{
 		err = t.CleanupTags()
 		if err != nil {
 			logger.Error("Failed to clean up tags", "error", err)
-			os.Exit(1)
+			return fmt.Errorf("failed to clean up tags: %w", err)
 		}
 
 		logger.Info("Tag cleanup completed successfully")
+		return nil
 	},
 }
 

cmd/cleanup_test.go

@@ -46,7 +46,7 @@ func TestCleanupCmd(t *testing.T) {
 			testCleanupCmd := &cobra.Command{
 				Use:   "cleanup",
 				Short: "cleanup removes all services with the tag prefix",
-				Run:   cleanupCmd.Run,
+				RunE:  cleanupCmd.RunE,
 			}
 			cmd.AddCommand(testCleanupCmd)
 
@@ -124,18 +124,120 @@ func TestCleanupCmdHelp(t *testing.T) {
 	testCleanupCmd := &cobra.Command{
 		Use:   "cleanup",
 		Short: "cleanup removes all services with the tag prefix from a given consul service",
-		Run:   cleanupCmd.Run,
+		RunE:  cleanupCmd.RunE,
 	}
 	cmd.AddCommand(testCleanupCmd)
 
 	buf := new(bytes.Buffer)
 	cmd.SetOut(buf)
 	cmd.SetArgs([]string{"cleanup", "--help"})
-	
+
 	err := cmd.Execute()
 	assert.NoError(t, err)
 
 	output := buf.String()
 	assert.Contains(t, output, "cleanup removes all services with the tag prefix")
 	assert.Contains(t, output, "Usage:")
-}
+}
+
+func TestCleanupCmdExecution(t *testing.T) {
+	tests := []struct {
+		name          string
+		consulAddr    string
+		expectError   bool
+		errorContains string
+	}{
+		{
+			name:          "Invalid consul address",
+			consulAddr:    "invalid-consul-address",
+			expectError:   true,
+			errorContains: "failed to clean up tags",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := &cobra.Command{Use: "tagit"}
+			cmd.PersistentFlags().StringP("consul-addr", "c", "127.0.0.1:8500", "consul address")
+			cmd.PersistentFlags().StringP("service-id", "s", "", "consul service id")
+			cmd.PersistentFlags().StringP("script", "x", "", "path to script used to generate tags")
+			cmd.PersistentFlags().StringP("tag-prefix", "p", "tagged", "prefix to be added to tags")
+			cmd.PersistentFlags().StringP("token", "t", "", "consul token")
+
+			testCleanupCmd := &cobra.Command{
+				Use:   "cleanup",
+				Short: "cleanup removes all services with the tag prefix from a given consul service",
+				RunE:  cleanupCmd.RunE,
+			}
+			cmd.AddCommand(testCleanupCmd)
+
+			var stderr bytes.Buffer
+			cmd.SetErr(&stderr)
+			cmd.SetArgs([]string{
+				"cleanup",
+				"--service-id=test-service",
+				"--script=/tmp/test.sh",
+				"--consul-addr=" + tt.consulAddr,
+				"--tag-prefix=test",
+			})
+
+			err := cmd.Execute()
+
+			if tt.expectError {
+				assert.Error(t, err)
+				if tt.errorContains != "" {
+					assert.Contains(t, err.Error(), tt.errorContains)
+				}
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestCleanupCmdFlagRetrieval(t *testing.T) {
+	// Test that all flag retrievals work correctly within the RunE function
+	cmd := &cobra.Command{Use: "tagit"}
+	cmd.PersistentFlags().StringP("consul-addr", "c", "127.0.0.1:8500", "consul address")
+	cmd.PersistentFlags().StringP("service-id", "s", "", "consul service id")
+	cmd.PersistentFlags().StringP("script", "x", "", "path to script used to generate tags")
+	cmd.PersistentFlags().StringP("tag-prefix", "p", "tagged", "prefix to be added to tags")
+	cmd.PersistentFlags().StringP("token", "t", "", "consul token")
+
+	var capturedValues map[string]string
+
+	testCleanupCmd := &cobra.Command{
+		Use:   "cleanup",
+		Short: "cleanup removes all services with the tag prefix from a given consul service",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			// Test the same flag access pattern used in the actual cleanup command
+			capturedValues = make(map[string]string)
+			capturedValues["consul-addr"] = cmd.InheritedFlags().Lookup("consul-addr").Value.String()
+			capturedValues["token"] = cmd.InheritedFlags().Lookup("token").Value.String()
+			capturedValues["service-id"] = cmd.InheritedFlags().Lookup("service-id").Value.String()
+			capturedValues["tag-prefix"] = cmd.InheritedFlags().Lookup("tag-prefix").Value.String()
+
+			// Don't actually try to connect to consul - just test flag access
+			return nil
+		},
+	}
+	cmd.AddCommand(testCleanupCmd)
+
+	cmd.SetArgs([]string{
+		"cleanup",
+		"--service-id=test-service",
+		"--script=/tmp/test.sh",
+		"--consul-addr=localhost:9500",
+		"--tag-prefix=test-prefix",
+		"--token=test-token",
+	})
+
+	err := cmd.Execute()
+	assert.NoError(t, err)
+
+	// Verify all values were captured correctly
+	assert.Equal(t, "localhost:9500", capturedValues["consul-addr"])
+	assert.Equal(t, "test-token", capturedValues["token"])
+	assert.Equal(t, "test-service", capturedValues["service-id"])
+	assert.Equal(t, "test-prefix", capturedValues["tag-prefix"])
+}

cmd/root_test.go

@@ -189,7 +189,7 @@ func TestRootCmdHelp(t *testing.T) {
 	var buf bytes.Buffer
 	rootCmd.SetOut(&buf)
 	rootCmd.SetArgs([]string{"--help"})
-	
+
 	err := rootCmd.Execute()
 	assert.NoError(t, err)
 
@@ -203,4 +203,4 @@ func TestRootCmdHelp(t *testing.T) {
 	assert.Contains(t, output, "cleanup")
 	assert.Contains(t, output, "run")
 	assert.Contains(t, output, "systemd")
-}
+}

cmd/run.go

@@ -17,6 +17,7 @@ package cmd
 
 import (
 	"context"
+	"fmt"
 	"log/slog"
 	"os"
 	"os/signal"
@@ -36,60 +37,60 @@ var runCmd = &cobra.Command{
 
 example: tagit run -s my-super-service -x '/tmp/tag-role.sh'
 `,
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{
 			Level: slog.LevelInfo,
 		}))
 
 		interval, err := cmd.InheritedFlags().GetString("interval")
 		if err != nil {
 			logger.Error("Failed to get interval flag", "error", err)
-			os.Exit(1)
+			return err
 		}
 
 		if interval == "" || interval == "0" {
 			logger.Error("Interval is required")
-			os.Exit(1)
+			return fmt.Errorf("interval is required and cannot be empty or zero")
 		}
 
 		validInterval, err := time.ParseDuration(interval)
 		if err != nil {
 			logger.Error("Invalid interval", "interval", interval, "error", err)
-			os.Exit(1)
+			return fmt.Errorf("invalid interval %q: %w", interval, err)
 		}
 
 		config := api.DefaultConfig()
 		config.Address, err = cmd.InheritedFlags().GetString("consul-addr")
 		if err != nil {
 			logger.Error("Failed to get consul-addr flag", "error", err)
-			os.Exit(1)
+			return err
 		}
 		config.Token, err = cmd.InheritedFlags().GetString("token")
 		if err != nil {
 			logger.Error("Failed to get token flag", "error", err)
-			os.Exit(1)
+			return err
 		}
 
 		consulClient, err := api.NewClient(config)
 		if err != nil {
 			logger.Error("Failed to create Consul client", "error", err)
-			os.Exit(1)
+			return fmt.Errorf("failed to create Consul client: %w", err)
 		}
 
 		serviceID, err := cmd.InheritedFlags().GetString("service-id")
 		if err != nil {
 			logger.Error("Failed to get service-id flag", "error", err)
-			os.Exit(1)
+			return err
 		}
 		script, err := cmd.InheritedFlags().GetString("script")
 		if err != nil {
 			logger.Error("Failed to get script flag", "error", err)
-			os.Exit(1)
+			return err
 		}
 		tagPrefix, err := cmd.InheritedFlags().GetString("tag-prefix")
 		if err != nil {
 			logger.Error("Failed to get tag-prefix flag", "error", err)
-			os.Exit(1)
+			return err
 		}
 
 		t := tagit.New(
@@ -124,6 +125,7 @@ example: tagit run -s my-super-service -x '/tmp/tag-role.sh'
 		t.Run(ctx)
 
 		logger.Info("Tagit has stopped")
+		return nil
 	},
 }