Change cmd setup to bubble up errors over exiting (#454)

* Change cmd setup to bubble up errors over exiting

* Update main to handle execute error

* Update changelog with PR #454

* Slight cleanup tweaks

CHANGELOG.md

@@ -8,6 +8,12 @@ This document is formatted according to the principles of [Keep A CHANGELOG](htt
 
 ---
 
+## [Unreleased]
+
+### Changed
+
+- Changed underlying cobra command setup to return errors instead of calling `os.Exit` directly to enable simpler testing. ([454](https://github.com/cucumber/godog/pull/454) - [mxygem])
+
 ## [v0.12.4]
 
 ### Added

cmd/godog/internal/cmd_build.go

@@ -3,7 +3,6 @@ package internal
 import (
 	"fmt"
 	"go/build"
-	"os"
 	"path/filepath"
 
 	"github.com/cucumber/godog/internal/builder"
@@ -28,7 +27,7 @@ package and contain buildable go source.
 The test runner can be executed with the same flags as when using godog run.`,
 		Example: `  godog build
   godog build -o ` + buildOutputDefault,
-		Run: buildCmdRunFunc,
+		RunE: buildCmdRunFunc,
 	}
 
 	buildCmd.Flags().StringVarP(&buildOutput, "output", "o", buildOutputDefault, `compiles the test runner to the named file
@@ -37,17 +36,15 @@ The test runner can be executed with the same flags as when using godog run.`,
 	return buildCmd
 }
 
-func buildCmdRunFunc(cmd *cobra.Command, args []string) {
+func buildCmdRunFunc(cmd *cobra.Command, args []string) error {
 	bin, err := filepath.Abs(buildOutput)
 	if err != nil {
-		fmt.Fprintln(os.Stderr, "could not locate absolute path for:", buildOutput, err)
-		os.Exit(1)
+		return fmt.Errorf("could not locate absolute path for: %q. reason: %v", buildOutput, err)
 	}
 
 	if err = builder.Build(bin); err != nil {
-		fmt.Fprintln(os.Stderr, "could not build binary at:", buildOutput, err)
-		os.Exit(1)
+		return fmt.Errorf("could not build binary at: %q. reason: %v", buildOutput, err)
 	}
 
-	os.Exit(0)
+	return nil
 }

cmd/godog/internal/cmd_root.go

@@ -1,9 +1,12 @@
 package internal
 
 import (
+	"fmt"
+
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
+	"github.com/cucumber/godog/colors"
 	"github.com/cucumber/godog/internal/flags"
 )
 
@@ -18,28 +21,33 @@ func CreateRootCmd() cobra.Command {
 Command should be run from the directory of tested package
 and contain buildable go source.`,
 		Args: cobra.ArbitraryArgs,
-
 		// Deprecated: Use godog build, godog run or godog version.
 		// This is to support the legacy direct usage of the root command.
-		Run: func(cmd *cobra.Command, args []string) {
-			if version {
-				versionCmdRunFunc(cmd, args)
-			}
-
-			if len(output) > 0 {
-				buildOutput = output
-				buildCmdRunFunc(cmd, args)
-			}
-
-			runCmdRunFunc(cmd, args)
-		},
+		RunE: runRootCmd,
 	}
 
 	bindRootCmdFlags(rootCmd.Flags())
 
 	return rootCmd
 }
 
+func runRootCmd(cmd *cobra.Command, args []string) error {
+	if version {
+		versionCmdRunFunc(cmd, args)
+		return nil
+	}
+
+	if len(output) > 0 {
+		buildOutput = output
+		if err := buildCmdRunFunc(cmd, args); err != nil {
+			return err
+		}
+	}
+
+	fmt.Println(colors.Yellow("Use of godog without a sub-command is deprecated. Please use godog build, godog run or godog version."))
+	return runCmdRunFunc(cmd, args)
+}
+
 func bindRootCmdFlags(flagSet *pflag.FlagSet) {
 	flagSet.StringVarP(&output, "output", "o", "", "compiles the test runner to the named file")
 	flagSet.BoolVar(&version, "version", false, "show current version")

cmd/godog/internal/cmd_run.go

@@ -32,72 +32,75 @@ buildable go source.`,
     feature (*.feature)
     scenario at specific line (*.feature:10)
   If no feature arguments are supplied, godog will use "features/" by default.`,
-		Run: runCmdRunFunc,
+		RunE: runCmdRunFunc,
 	}
 
 	flags.BindRunCmdFlags("", runCmd.Flags(), &opts)
 
 	return runCmd
 }
 
-func runCmdRunFunc(cmd *cobra.Command, args []string) {
+func runCmdRunFunc(cmd *cobra.Command, args []string) error {
 	osArgs := os.Args[1:]
 
 	if len(osArgs) > 0 && osArgs[0] == "run" {
 		osArgs = osArgs[1:]
 	}
 
-	status, err := buildAndRunGodog(osArgs)
-	if err != nil {
-		fmt.Fprintln(os.Stderr, err)
-		os.Exit(1)
+	if err := buildAndRunGodog(osArgs); err != nil {
+		return err
 	}
 
-	os.Exit(status)
+	return nil
 }
 
-func buildAndRunGodog(args []string) (_ int, err error) {
+func buildAndRunGodog(args []string) (err error) {
 	bin, err := filepath.Abs(buildOutputDefault)
 	if err != nil {
-		return 1, err
+		return err
 	}
 
 	if err = builder.Build(bin); err != nil {
-		return 1, err
+		return err
 	}
 
 	defer os.Remove(bin)
 
 	return runGodog(bin, args)
 }
 
-func runGodog(bin string, args []string) (_ int, err error) {
+func runGodog(bin string, args []string) (err error) {
 	cmd := exec.Command(bin, args...)
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 	cmd.Stdin = os.Stdin
 	cmd.Env = os.Environ()
 
 	if err = cmd.Start(); err != nil {
-		return 0, err
+		return err
 	}
 
 	if err = cmd.Wait(); err == nil {
-		return 0, nil
+		return nil
 	}
 
 	exiterr, ok := err.(*exec.ExitError)
 	if !ok {
-		return 0, err
+		return err
+	}
+
+	st, ok := exiterr.Sys().(syscall.WaitStatus)
+	if !ok {
+		return fmt.Errorf("failed to convert error to syscall wait status. original error: %w", exiterr)
 	}
 
 	// This works on both Unix and Windows. Although package
 	// syscall is generally platform dependent, WaitStatus is
 	// defined for both Unix and Windows and in both cases has
 	// an ExitStatus() method with the same signature.
-	if st, ok := exiterr.Sys().(syscall.WaitStatus); ok {
-		return st.ExitStatus(), nil
+	if st.ExitStatus() > 0 {
+		return err
 	}
 
-	return 1, nil
+	return nil
 }

cmd/godog/internal/cmd_version.go

@@ -12,16 +12,14 @@ import (
 // CreateVersionCmd creates the version subcommand.
 func CreateVersionCmd() cobra.Command {
 	versionCmd := cobra.Command{
-		Use:                   "version",
-		Short:                 "Show current version",
-		Run:                   versionCmdRunFunc,
-		DisableFlagsInUseLine: true,
+		Use:     "version",
+		Short:   "Show current version",
+		Version: godog.Version,
 	}
 
 	return versionCmd
 }
 
 func versionCmdRunFunc(cmd *cobra.Command, args []string) {
 	fmt.Fprintln(os.Stdout, "Godog version is:", godog.Version)
-	os.Exit(0)
 }

cmd/godog/main.go

@@ -1,6 +1,9 @@
 package main
 
 import (
+	"fmt"
+	"os"
+
 	"github.com/cucumber/godog/cmd/godog/internal"
 )
 
@@ -11,5 +14,9 @@ func main() {
 	versionCmd := internal.CreateVersionCmd()
 
 	rootCmd.AddCommand(&buildCmd, &runCmd, &versionCmd)
-	rootCmd.Execute()
+
+	if err := rootCmd.Execute(); err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
 }