From 5001c4f4fe988e2b0de23a6f60eeca8f34c0ba48 Mon Sep 17 00:00:00 2001 From: Gemini Smith Date: Tue, 25 Jan 2022 16:52:57 -0700 Subject: [PATCH] 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 | 6 +++++ cmd/godog/internal/cmd_build.go | 13 +++++------ cmd/godog/internal/cmd_root.go | 34 +++++++++++++++++----------- cmd/godog/internal/cmd_run.go | 37 +++++++++++++++++-------------- cmd/godog/internal/cmd_version.go | 8 +++---- cmd/godog/main.go | 9 +++++++- 6 files changed, 63 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ca38e5..6348fa4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cmd/godog/internal/cmd_build.go b/cmd/godog/internal/cmd_build.go index 709a7a8..3175666 100644 --- a/cmd/godog/internal/cmd_build.go +++ b/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 } diff --git a/cmd/godog/internal/cmd_root.go b/cmd/godog/internal/cmd_root.go index 34b9918..c25c656 100644 --- a/cmd/godog/internal/cmd_root.go +++ b/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,21 +21,9 @@ 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()) @@ -40,6 +31,23 @@ and contain buildable go source.`, 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") diff --git a/cmd/godog/internal/cmd_run.go b/cmd/godog/internal/cmd_run.go index d146522..52fde46 100644 --- a/cmd/godog/internal/cmd_run.go +++ b/cmd/godog/internal/cmd_run.go @@ -32,7 +32,7 @@ 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) @@ -40,30 +40,28 @@ buildable go source.`, 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) @@ -71,7 +69,7 @@ func buildAndRunGodog(args []string) (_ int, err error) { 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 @@ -79,25 +77,30 @@ func runGodog(bin string, args []string) (_ int, err error) { 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 } diff --git a/cmd/godog/internal/cmd_version.go b/cmd/godog/internal/cmd_version.go index 98d1b78..e0423f3 100644 --- a/cmd/godog/internal/cmd_version.go +++ b/cmd/godog/internal/cmd_version.go @@ -12,10 +12,9 @@ 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 @@ -23,5 +22,4 @@ func CreateVersionCmd() cobra.Command { func versionCmdRunFunc(cmd *cobra.Command, args []string) { fmt.Fprintln(os.Stdout, "Godog version is:", godog.Version) - os.Exit(0) } diff --git a/cmd/godog/main.go b/cmd/godog/main.go index afda6c4..99b8730 100644 --- a/cmd/godog/main.go +++ b/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) + } }