diff --git a/CHANGELOG.md b/CHANGELOG.md index 58a9f6c..941416d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ All notable changes to `atb-cli` are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/). +## [Unreleased] + +### Fixed + +- Every error was printed twice. Cobra wrote the `Error: ...` line when a + command returned and `main` wrote the same sentence again, so one failure + looked like two. The error is now reported once, keeping the `Error: ` prefix + that separates it from the progress and warning output `atb` also writes to + stderr. Exit codes are unchanged. + ## [v0.21.0](https://github.com/allthebacteria/atb-cli/releases/tag/v0.21.0) - 2026-08-05 ### Added diff --git a/cmd/atb/main.go b/cmd/atb/main.go index 664b78e..f1aec81 100644 --- a/cmd/atb/main.go +++ b/cmd/atb/main.go @@ -2,8 +2,11 @@ package main import ( "fmt" + "io" "os" + "github.com/spf13/cobra" + "github.com/allthebacteria/atb-cli/internal/cli" ) @@ -12,7 +15,13 @@ var version = "dev" func main() { cli.RootCmd.Version = version - err := cli.RootCmd.Execute() + os.Exit(run(cli.RootCmd, os.Stderr)) +} + +// run executes root and returns the process exit code, reporting any error on +// stderr. +func run(root *cobra.Command, stderr io.Writer) int { + err := root.Execute() // Let the background update check finish saving state (up to 2s). if cli.WaitForUpdateCheck != nil { @@ -20,7 +29,8 @@ func main() { } if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) + fmt.Fprintln(stderr, "Error:", err) + return 1 } + return 0 } diff --git a/cmd/atb/main_test.go b/cmd/atb/main_test.go new file mode 100644 index 0000000..1474b49 --- /dev/null +++ b/cmd/atb/main_test.go @@ -0,0 +1,67 @@ +package main + +import ( + "bytes" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/allthebacteria/atb-cli/internal/cli" +) + +// failingRoot builds a real command tree and an argument list that always +// fails during flag parsing, so the test does not depend on any data files. +func failingRoot() (*cobra.Command, *bytes.Buffer) { + root := cli.NewRootCmd("test") + root.SetArgs([]string{"query", "--no-such-flag"}) + + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + return root, &out +} + +// Cobra reports the error when Execute returns and main reported it a second +// time, so the same sentence reached the user twice. +func TestRunReportsAnErrorOnce(t *testing.T) { + root, out := failingRoot() + + code := run(root, out) + + if code != 1 { + t.Errorf("exit code: got %d, want 1", code) + } + if got := strings.Count(out.String(), "unknown flag: --no-such-flag"); got != 1 { + t.Errorf("error reported %d time(s), want 1:\n%s", got, out.String()) + } +} + +// atb writes progress and warnings to stderr as well, so the error keeps the +// conventional prefix that separates it from that output. +func TestRunPrefixesTheError(t *testing.T) { + root, out := failingRoot() + + run(root, out) + + line := strings.TrimSpace(out.String()) + if !strings.HasPrefix(line, "Error: ") { + t.Errorf("error line should start with %q, got:\n%s", "Error: ", line) + } +} + +func TestRunReportsNothingOnSuccess(t *testing.T) { + root := cli.NewRootCmd("test") + root.SetArgs([]string{"--help"}) + + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + + if code := run(root, &out); code != 0 { + t.Errorf("exit code: got %d, want 0", code) + } + if strings.Contains(out.String(), "Error:") { + t.Errorf("--help should not report an error, got:\n%s", out.String()) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 517f93f..f16835c 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -26,10 +26,14 @@ var ( ) // RootCmd is the base command for atb. +// +// SilenceErrors leaves reporting to main, which prints the error after the +// background update check has finished writing to stderr. var RootCmd = &cobra.Command{ - Use: "atb", - Short: "Query and download AllTheBacteria genomes", - SilenceUsage: true, + Use: "atb", + Short: "Query and download AllTheBacteria genomes", + SilenceUsage: true, + SilenceErrors: true, } func init() { @@ -69,10 +73,11 @@ func NewRootCmd(version string) *cobra.Command { var localCfgFile, localDataDir string root := &cobra.Command{ - Use: "atb", - Short: "Query and download AllTheBacteria genomes", - SilenceUsage: true, - Version: version, + Use: "atb", + Short: "Query and download AllTheBacteria genomes", + SilenceUsage: true, + SilenceErrors: true, + Version: version, } root.PersistentFlags().StringVar(&localCfgFile, "config", "", "config file (default $HOME/.atb/config.toml)")