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

Author: ncode

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,7 +124,7 @@ 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)
 

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
 	},
 }
 

cmd/run_test.go

@@ -34,6 +34,12 @@ func TestRunCmd(t *testing.T) {
 			expectError:   true,
 			errorContains: "required flag(s) \"script\" not set",
 		},
+		{
+			name:          "Invalid interval format",
+			args:          []string{"run", "--service-id=test-service", "--script=/tmp/test.sh", "--interval=invalid"},
+			expectError:   true,
+			errorContains: "invalid interval",
+		},
 	}
 
 	for _, tt := range tests {
@@ -53,7 +59,7 @@ func TestRunCmd(t *testing.T) {
 			testRunCmd := &cobra.Command{
 				Use:   "run",
 				Short: "Run tagit",
-				Run:   runCmd.Run,
+				RunE:  runCmd.RunE,
 			}
 			cmd.AddCommand(testRunCmd)