From 3d2675a768020e0b58d9d1e62e27b8aa291824ca Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Sun, 9 Nov 2025 15:59:29 -0700 Subject: [PATCH 1/9] Fix handling of --help arg for sub commands --- cmd/src/cmd.go | 54 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/cmd/src/cmd.go b/cmd/src/cmd.go index 95c16e6e0e..8507733773 100644 --- a/cmd/src/cmd.go +++ b/cmd/src/cmd.go @@ -6,6 +6,7 @@ import ( "log" "os" "slices" + "strings" "github.com/sourcegraph/src-cli/internal/cmderrors" ) @@ -44,16 +45,32 @@ type commander []*command // run runs the command. func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args []string) { + + // Check if --help args are anywhere in the command; if yes, then + // remove it from the list of args at this point to avoid interrupting recursive function calls, + // and append it to the deepest command / subcommand + filteredArgs := make([]string, 0, len(args)) + helpFlags := []string{"help", "h"} + helpRequested := false + for _, arg := range args { + trimmedArg := strings.TrimLeft(arg, "-") + if slices.Contains(helpFlags, trimmedArg) { + helpRequested = true + } else { + filteredArgs = append(filteredArgs, arg) + } + } + // Parse flags. flagSet.Usage = func() { _, _ = fmt.Fprint(flag.CommandLine.Output(), usageText) } if !flagSet.Parsed() { - _ = flagSet.Parse(args) + _ = flagSet.Parse(filteredArgs) } - // Print usage if the command is "help". - if flagSet.Arg(0) == "help" || flagSet.NArg() == 0 { + // If no subcommands remain (or help requested with no subcommands), print usage. + if flagSet.NArg() == 0 { flagSet.SetOutput(os.Stdout) flagSet.Usage() os.Exit(0) @@ -62,10 +79,16 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] // Configure default usage funcs for commands. for _, cmd := range c { cmd := cmd + + // If the command / subcommand has defined its own usageFunc, then use it if cmd.usageFunc != nil { cmd.flagSet.Usage = cmd.usageFunc continue } + + // If the command / subcommand has not defined its own usageFunc, + // then generate a basic default usageFunc, + // using the command's defined flagSet and their defaults cmd.flagSet.Usage = func() { _, _ = fmt.Fprintf(flag.CommandLine.Output(), "Usage of '%s %s':\n", cmdName, cmd.flagSet.Name()) cmd.flagSet.PrintDefaults() @@ -86,25 +109,18 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] log.Fatal("reading config: ", err) } - // Print help to stdout if requested - if slices.IndexFunc(args, func(s string) bool { - return s == "--help" - }) >= 0 { - cmd.flagSet.SetOutput(os.Stdout) - flag.CommandLine.SetOutput(os.Stdout) - cmd.flagSet.Usage() - os.Exit(0) - } - - // Parse subcommand flags. + // Get subcommand args, and re-add help flag if it was requested args := flagSet.Args()[1:] - if err := cmd.flagSet.Parse(args); err != nil { - fmt.Printf("Error parsing subcommand flags: %s\n", err) - panic(fmt.Sprintf("all registered commands should use flag.ExitOnError: error: %s", err)) + if helpRequested { + args = append(args, "-h") } - // Execute the subcommand. - if err := cmd.handler(flagSet.Args()[1:]); err != nil { + // Set output to stdout for help (flag package defaults to stderr) + cmd.flagSet.SetOutput(os.Stdout) + flag.CommandLine.SetOutput(os.Stdout) + + // Execute the subcommand + if err := cmd.handler(args); err != nil { if _, ok := err.(*cmderrors.UsageError); ok { log.Printf("error: %s\n\n", err) cmd.flagSet.SetOutput(os.Stderr) From 34dd638d96bb107c07bb90921104785046a46f39 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Sun, 9 Nov 2025 16:08:32 -0700 Subject: [PATCH 2/9] Keep helpFlags explicit, to avoid possible interference with other args, ex file names "h" --- cmd/src/cmd.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/cmd/src/cmd.go b/cmd/src/cmd.go index 8507733773..f2e46ee6a3 100644 --- a/cmd/src/cmd.go +++ b/cmd/src/cmd.go @@ -6,7 +6,6 @@ import ( "log" "os" "slices" - "strings" "github.com/sourcegraph/src-cli/internal/cmderrors" ) @@ -50,11 +49,18 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] // remove it from the list of args at this point to avoid interrupting recursive function calls, // and append it to the deepest command / subcommand filteredArgs := make([]string, 0, len(args)) - helpFlags := []string{"help", "h"} helpRequested := false + + helpFlags := []string{ + "--h", + "--help", + "-h", + "-help", + "help", + } + for _, arg := range args { - trimmedArg := strings.TrimLeft(arg, "-") - if slices.Contains(helpFlags, trimmedArg) { + if slices.Contains(helpFlags, arg) { helpRequested = true } else { filteredArgs = append(filteredArgs, arg) From 0f69582ffa347f9be17e83534d135ebd604d0ba9 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Sun, 9 Nov 2025 17:54:16 -0700 Subject: [PATCH 3/9] Currently broken, not sure what's up --- cmd/src/cmd.go | 47 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/cmd/src/cmd.go b/cmd/src/cmd.go index f2e46ee6a3..2b435856f9 100644 --- a/cmd/src/cmd.go +++ b/cmd/src/cmd.go @@ -42,7 +42,7 @@ func (c *command) matches(name string) bool { // commander represents a top-level command with subcommands. type commander []*command -// run runs the command. +// Run the command func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args []string) { // Check if --help args are anywhere in the command; if yes, then @@ -67,22 +67,24 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] } } - // Parse flags. + // Define the usage function from usageText flagSet.Usage = func() { _, _ = fmt.Fprint(flag.CommandLine.Output(), usageText) } + + // Parse the command's flags, if not already parsed if !flagSet.Parsed() { _ = flagSet.Parse(filteredArgs) } - // If no subcommands remain (or help requested with no subcommands), print usage. + // If no subcommands remain (or help requested with no subcommands), print usage if flagSet.NArg() == 0 { flagSet.SetOutput(os.Stdout) flagSet.Usage() os.Exit(0) } - // Configure default usage funcs for commands. + // Configure default usage funcs for all commands. for _, cmd := range c { cmd := cmd @@ -101,32 +103,51 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] } } - // Find the subcommand to execute. + // Find the subcommand to execute + // Assume the subcommand is the first arg in the flagSet name := flagSet.Arg(0) + + // Loop through the list of all registered subcommands for _, cmd := range c { + + // If the first arg is not this registered commmand in the loop, try the next registered command if !cmd.matches(name) { continue } + // If the first arg is this registered commmand in the loop, then try and run it, then exit - // Read global configuration now. + // Read global configuration var err error cfg, err = readConfig() if err != nil { log.Fatal("reading config: ", err) } - // Get subcommand args, and re-add help flag if it was requested + // Get the remaining args, to pass to the subcommand, as an unparsed array of previously parsed args args := flagSet.Args()[1:] - if helpRequested { - args = append(args, "-h") - } // Set output to stdout for help (flag package defaults to stderr) cmd.flagSet.SetOutput(os.Stdout) flag.CommandLine.SetOutput(os.Stdout) + // Note: We can't parse flags here because commanders need to pass unparsed args to subcommand handlers + // Each handler is responsible for parsing its own flags + // All commands must use `flagSet := flag.NewFlagSet("", flag.ExitOnError)` to ensure usage helper text is printed automatically on arg parse errors + // Parse the subcommand's args, on its behalf, to test if flag.ExitOnError is not set + // if err := cmd.flagSet.Parse(args); err != nil { + // fmt.Printf("Error parsing subcommand flags: %s\n", err) + // panic(fmt.Sprintf("all registered commands should use flag.ExitOnError: error: %s", err)) + // } + + // If the --help arg was provided, re-add it here for the lowest command to parse and action + if helpRequested { + args = append(args, "-h") + } + // Execute the subcommand if err := cmd.handler(args); err != nil { + + // If the subcommand returns a UsageError, then print the error and usage helper text if _, ok := err.(*cmderrors.UsageError); ok { log.Printf("error: %s\n\n", err) cmd.flagSet.SetOutput(os.Stderr) @@ -134,6 +155,8 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] cmd.flagSet.Usage() os.Exit(2) } + + // If the subcommand returns any other error, then print the error if e, ok := err.(*cmderrors.ExitCodeError); ok { if e.HasError() { log.Println(e) @@ -142,8 +165,12 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] } log.Fatal(err) } + + // If no error was returned, then exit the application os.Exit(0) } + + // If the first arg didn't match any registered commands, print errors and exit the application log.Printf("%s: unknown subcommand %q", cmdName, name) log.Fatalf("Run '%s help' for usage.", cmdName) } From 99a51d804c4ebfc0d31f05eeece3f500d79595d5 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Sun, 9 Nov 2025 19:14:47 -0700 Subject: [PATCH 4/9] Deduplicated usageFunc loop, re-added flag parse check, clarified comments --- cmd/src/cmd.go | 91 +++++++++++++++++++++++--------------------------- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/cmd/src/cmd.go b/cmd/src/cmd.go index 2b435856f9..6078022706 100644 --- a/cmd/src/cmd.go +++ b/cmd/src/cmd.go @@ -45,18 +45,19 @@ type commander []*command // Run the command func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args []string) { - // Check if --help args are anywhere in the command; if yes, then - // remove it from the list of args at this point to avoid interrupting recursive function calls, - // and append it to the deepest command / subcommand + // Check if --help args are anywhere in the command + // If yes, then remove it from the list of args at this point, + // then append it to the deepest command / subcommand, later, + // to avoid outputting usage text for a commander when a subcommand is specified filteredArgs := make([]string, 0, len(args)) helpRequested := false helpFlags := []string{ - "--h", + "help", + "-help", "--help", "-h", - "-help", - "help", + "--h", } for _, arg := range args { @@ -67,44 +68,19 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] } } - // Define the usage function from usageText + // Define the usage function for the commander flagSet.Usage = func() { _, _ = fmt.Fprint(flag.CommandLine.Output(), usageText) } - // Parse the command's flags, if not already parsed + // Parse the commander's flags, if not already parsed if !flagSet.Parsed() { _ = flagSet.Parse(filteredArgs) } - // If no subcommands remain (or help requested with no subcommands), print usage - if flagSet.NArg() == 0 { - flagSet.SetOutput(os.Stdout) - flagSet.Usage() - os.Exit(0) - } - - // Configure default usage funcs for all commands. - for _, cmd := range c { - cmd := cmd - - // If the command / subcommand has defined its own usageFunc, then use it - if cmd.usageFunc != nil { - cmd.flagSet.Usage = cmd.usageFunc - continue - } - - // If the command / subcommand has not defined its own usageFunc, - // then generate a basic default usageFunc, - // using the command's defined flagSet and their defaults - cmd.flagSet.Usage = func() { - _, _ = fmt.Fprintf(flag.CommandLine.Output(), "Usage of '%s %s':\n", cmdName, cmd.flagSet.Name()) - cmd.flagSet.PrintDefaults() - } - } - // Find the subcommand to execute - // Assume the subcommand is the first arg in the flagSet + // This assumes the subcommand is the first arg in the flagSet, + // i.e. any global args have been removed from the flagSet name := flagSet.Arg(0) // Loop through the list of all registered subcommands @@ -116,6 +92,20 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] } // If the first arg is this registered commmand in the loop, then try and run it, then exit + // Set up the usage function for this subcommand + // If the subcommand has a usageFunc defined, then use it + if cmd.usageFunc != nil { + cmd.flagSet.Usage = cmd.usageFunc + } else { + // If the subcommand does not have a usageFunc defined, + // then define a simple default one, + // using the list of flags defined in the subcommand, and their description strings + cmd.flagSet.Usage = func() { + _, _ = fmt.Fprintf(flag.CommandLine.Output(), "Usage of '%s %s':\n", cmdName, cmd.flagSet.Name()) + cmd.flagSet.PrintDefaults() + } + } + // Read global configuration var err error cfg, err = readConfig() @@ -123,32 +113,32 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] log.Fatal("reading config: ", err) } - // Get the remaining args, to pass to the subcommand, as an unparsed array of previously parsed args + // Get the remainder of the args, excluding the first arg / this command name args := flagSet.Args()[1:] - // Set output to stdout for help (flag package defaults to stderr) + // Set output to stdout, for usage / helper text printed for the --help flag (flag package defaults to stderr) cmd.flagSet.SetOutput(os.Stdout) flag.CommandLine.SetOutput(os.Stdout) - // Note: We can't parse flags here because commanders need to pass unparsed args to subcommand handlers - // Each handler is responsible for parsing its own flags - // All commands must use `flagSet := flag.NewFlagSet("", flag.ExitOnError)` to ensure usage helper text is printed automatically on arg parse errors - // Parse the subcommand's args, on its behalf, to test if flag.ExitOnError is not set - // if err := cmd.flagSet.Parse(args); err != nil { - // fmt.Printf("Error parsing subcommand flags: %s\n", err) - // panic(fmt.Sprintf("all registered commands should use flag.ExitOnError: error: %s", err)) - // } - // If the --help arg was provided, re-add it here for the lowest command to parse and action if helpRequested { args = append(args, "-h") } + // Parse the subcommand's args, on its behalf, to test and ensure flag.ExitOnError is set + // just in case any future authors of subcommands forget to set flag.ExitOnError + if err := cmd.flagSet.Parse(args); err != nil { + fmt.Printf("Error parsing subcommand flags: %s\n", err) + panic(fmt.Sprintf("all registered commands should use flag.ExitOnError: error: %s", err)) + } + // Execute the subcommand + // Handle any errors returned if err := cmd.handler(args); err != nil { - // If the subcommand returns a UsageError, then print the error and usage helper text + // If the returned error is of type UsageError if _, ok := err.(*cmderrors.UsageError); ok { + // then print the error and usage helper text, both to stderr log.Printf("error: %s\n\n", err) cmd.flagSet.SetOutput(os.Stderr) flag.CommandLine.SetOutput(os.Stderr) @@ -156,17 +146,20 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] os.Exit(2) } - // If the subcommand returns any other error, then print the error + // If the returned error is of type ExitCodeError if e, ok := err.(*cmderrors.ExitCodeError); ok { + // Then log the error and exit with the exit code if e.HasError() { log.Println(e) } os.Exit(e.Code()) } + + // For all other types of errors, log them as fatal, and exit log.Fatal(err) } - // If no error was returned, then exit the application + // If no error was returned, then just exit the application cleanly os.Exit(0) } From d243ce8ef74e23d4ce81e19a48b4e90ed63de37e Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Mon, 10 Nov 2025 01:32:16 -0700 Subject: [PATCH 5/9] Finally passing --- cmd/src/cmd.go | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/cmd/src/cmd.go b/cmd/src/cmd.go index 6078022706..addf2295e6 100644 --- a/cmd/src/cmd.go +++ b/cmd/src/cmd.go @@ -93,10 +93,10 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] // If the first arg is this registered commmand in the loop, then try and run it, then exit // Set up the usage function for this subcommand - // If the subcommand has a usageFunc defined, then use it if cmd.usageFunc != nil { + // If the subcommand has a usageFunc defined, then use it cmd.flagSet.Usage = cmd.usageFunc - } else { + } else { // If the subcommand does not have a usageFunc defined, // then define a simple default one, // using the list of flags defined in the subcommand, and their description strings @@ -163,7 +163,23 @@ func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args [] os.Exit(0) } - // If the first arg didn't match any registered commands, print errors and exit the application - log.Printf("%s: unknown subcommand %q", cmdName, name) - log.Fatalf("Run '%s help' for usage.", cmdName) + // To make it after the big loop, that means name didn't match any registered commands + if name != "" { + log.Printf("%s: unknown command %q", cmdName, name) + flagSet.Usage() + os.Exit(2) + } + + // Special case to handle --help usage text for src command + if helpRequested { + // Set output to stdout, for usage / helper text printed for the --help flag (flag package defaults to stderr) + flagSet.SetOutput(os.Stdout) + flagSet.Usage() + os.Exit(0) + } + + // Special case to handle src command with no args + flagSet.Usage() + os.Exit(2) + } From 0378de50bd59e8409e59953eca7658d184a94c48 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Mon, 10 Nov 2025 01:34:18 -0700 Subject: [PATCH 6/9] Add cautionary tale --- cmd/src/cmd.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/src/cmd.go b/cmd/src/cmd.go index addf2295e6..3a92fb497c 100644 --- a/cmd/src/cmd.go +++ b/cmd/src/cmd.go @@ -45,6 +45,9 @@ type commander []*command // Run the command func (c commander) run(flagSet *flag.FlagSet, cmdName, usageText string, args []string) { + // NOTE: This function is quite brittle + // Especially with printing helper text at all 3 different levels of depth + // Check if --help args are anywhere in the command // If yes, then remove it from the list of args at this point, // then append it to the deepest command / subcommand, later, From b4d47134a5bd39f94ec25de4dd37dcf0d8f1bde4 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Mon, 10 Nov 2025 02:52:58 -0700 Subject: [PATCH 7/9] Adding tests --- cmd/src/cmd_test.go | 292 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 292 insertions(+) create mode 100644 cmd/src/cmd_test.go diff --git a/cmd/src/cmd_test.go b/cmd/src/cmd_test.go new file mode 100644 index 0000000000..2331298681 --- /dev/null +++ b/cmd/src/cmd_test.go @@ -0,0 +1,292 @@ +package main + +import ( + "bytes" + "flag" + "os" + "os/exec" + "testing" + + "github.com/sourcegraph/src-cli/internal/cmderrors" +) + +func TestCommand_Matches(t *testing.T) { + tests := []struct { + name string + cmd *command + input string + expected bool + }{ + { + name: "matches command name", + cmd: &command{ + flagSet: flag.NewFlagSet("test", flag.ExitOnError), + }, + input: "test", + expected: true, + }, + { + name: "matches alias", + cmd: &command{ + flagSet: flag.NewFlagSet("test", flag.ExitOnError), + aliases: []string{"t", "tst"}, + }, + input: "t", + expected: true, + }, + { + name: "matches second alias", + cmd: &command{ + flagSet: flag.NewFlagSet("test", flag.ExitOnError), + aliases: []string{"t", "tst"}, + }, + input: "tst", + expected: true, + }, + { + name: "no match", + cmd: &command{ + flagSet: flag.NewFlagSet("test", flag.ExitOnError), + aliases: []string{"t"}, + }, + input: "other", + expected: false, + }, + { + name: "empty string no match", + cmd: &command{ + flagSet: flag.NewFlagSet("test", flag.ExitOnError), + }, + input: "", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.cmd.matches(tt.input) + if result != tt.expected { + t.Errorf("matches(%q) = %v, want %v", tt.input, result, tt.expected) + } + }) + } +} + +func TestCommander_Run_ErrorHandling(t *testing.T) { + tests := []struct { + name string + handlerError error + expectedExit int + description string + }{ + { + name: "usage error", + handlerError: cmderrors.Usage("invalid usage"), + expectedExit: 2, + description: "should exit with code 2 for usage errors", + }, + { + name: "exit code error without message", + handlerError: cmderrors.ExitCode(42, nil), + expectedExit: 42, + description: "should exit with custom exit code", + }, + { + name: "exit code error with message", + handlerError: cmderrors.ExitCode(1, cmderrors.Usage("command failed")), + expectedExit: 1, + description: "should exit with custom exit code and log error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Logf("Test case: %s", tt.description) + }) + } +} + +func TestCommander_Run_UnknownCommand(t *testing.T) { + if os.Getenv("TEST_SUBPROCESS") == "1" { + testHomeDir = os.Getenv("TEST_TEMP_DIR") + cmdr := commander{ + &command{ + flagSet: flag.NewFlagSet("version", flag.ContinueOnError), + handler: func(args []string) error { return nil }, + }, + } + flagSet := flag.NewFlagSet("test", flag.ContinueOnError) + cmdr.run(flagSet, "src", "usage text", []string{"beans"}) + return + } + + tempDir := t.TempDir() + cmd := exec.Command(os.Args[0], "-test.run=^TestCommander_Run_UnknownCommand$") + cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1", "TEST_TEMP_DIR="+tempDir) + var stderr bytes.Buffer + cmd.Stderr = &stderr + err := cmd.Run() + + if err == nil { + t.Fatal("expected command to exit with non-zero code") + } + + if e, ok := err.(*exec.ExitError); ok { + if e.ExitCode() != 2 { + t.Errorf("expected exit code 2 for unknown command, got %d\nstderr: %s", e.ExitCode(), stderr.String()) + } + } else { + t.Errorf("unexpected error type: %v", err) + } +} + +func TestCommander_Run_HelpFlag(t *testing.T) { + if os.Getenv("TEST_SUBPROCESS") == "1" { + testHomeDir = os.Getenv("TEST_TEMP_DIR") + arg := os.Getenv("TEST_ARG") + cmdr := commander{} + flagSet := flag.NewFlagSet("test", flag.ContinueOnError) + cmdr.run(flagSet, "src", "usage text", []string{arg}) + return + } + + tests := []struct { + name string + arg string + contains string + expectedExit int + }{ + { + name: "help flag at root", + arg: "--help", + contains: "usage text", + expectedExit: 0, + }, + { + name: "-h flag at root", + arg: "-h", + contains: "usage text", + expectedExit: 0, + }, + { + name: "help command at root", + arg: "help", + contains: "usage text", + expectedExit: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + cmd := exec.Command(os.Args[0], "-test.run=^TestCommander_Run_HelpFlag$") + cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1", "TEST_TEMP_DIR="+tempDir, "TEST_ARG="+tt.arg) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() + + output := stdout.String() + stderr.String() + + if tt.expectedExit == 0 && err != nil { + t.Errorf("expected success, got error: %v\noutput: %s", err, output) + } else if tt.expectedExit != 0 { + if err == nil { + t.Errorf("expected exit code %d, got success", tt.expectedExit) + } else if e, ok := err.(*exec.ExitError); ok && e.ExitCode() != tt.expectedExit { + t.Errorf("expected exit code %d, got %d\noutput: %s", tt.expectedExit, e.ExitCode(), output) + } + } + + if !bytes.Contains([]byte(output), []byte(tt.contains)) { + t.Errorf("expected output to contain %q, got:\n%s", tt.contains, output) + } + }) + } +} + +func TestCommander_Run_NestedHelpFlags(t *testing.T) { + t.Skip("Complex nested help flag testing requires integration with actual src commands") +} + +func TestCommander_Run_InvalidSubcommand(t *testing.T) { + if os.Getenv("TEST_SUBPROCESS") == "1" { + testHomeDir = os.Getenv("TEST_TEMP_DIR") + arg := os.Getenv("TEST_ARG") + cmdr := commander{ + &command{ + flagSet: flag.NewFlagSet("version", flag.ContinueOnError), + handler: func(args []string) error { return nil }, + }, + } + flagSet := flag.NewFlagSet("test", flag.ContinueOnError) + cmdr.run(flagSet, "src", "root usage", []string{arg}) + return + } + + tests := []struct { + name string + arg string + expectedExit int + }{ + {"invalid root command", "beans", 2}, + {"invalid root with help", "foobar", 2}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + cmd := exec.Command(os.Args[0], "-test.run=^TestCommander_Run_InvalidSubcommand$") + cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1", "TEST_TEMP_DIR="+tempDir, "TEST_ARG="+tt.arg) + var stderr bytes.Buffer + cmd.Stderr = &stderr + err := cmd.Run() + + if err == nil { + t.Fatalf("expected exit code %d, got success", tt.expectedExit) + } + + if e, ok := err.(*exec.ExitError); ok { + if e.ExitCode() != tt.expectedExit { + t.Errorf("expected exit code %d, got %d\nstderr: %s", tt.expectedExit, e.ExitCode(), stderr.String()) + } + } else { + t.Errorf("unexpected error type: %v", err) + } + }) + } +} + +func TestCommander_Run_MissingRequiredArgs(t *testing.T) { + if os.Getenv("TEST_SUBPROCESS") == "1" { + testHomeDir = os.Getenv("TEST_TEMP_DIR") + cmdr := commander{ + &command{ + flagSet: flag.NewFlagSet("version", flag.ContinueOnError), + handler: func(args []string) error { return nil }, + }, + } + flagSet := flag.NewFlagSet("test", flag.ContinueOnError) + cmdr.run(flagSet, "src", "root usage", []string{}) + return + } + + tempDir := t.TempDir() + cmd := exec.Command(os.Args[0], "-test.run=^TestCommander_Run_MissingRequiredArgs$") + cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1", "TEST_TEMP_DIR="+tempDir) + var stderr bytes.Buffer + cmd.Stderr = &stderr + err := cmd.Run() + + if err == nil { + t.Fatal("expected exit code 2, got success") + } + + if e, ok := err.(*exec.ExitError); ok { + if e.ExitCode() != 2 { + t.Errorf("expected exit code 2, got %d\nstderr: %s", e.ExitCode(), stderr.String()) + } + } else { + t.Errorf("unexpected error type: %v", err) + } +} From 794e0b6f84a4515776ec35b11d4aed69314286f5 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Mon, 10 Nov 2025 03:04:50 -0700 Subject: [PATCH 8/9] Fix lint issue --- cmd/src/cmd_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/src/cmd_test.go b/cmd/src/cmd_test.go index 2331298681..d7181cea8c 100644 --- a/cmd/src/cmd_test.go +++ b/cmd/src/cmd_test.go @@ -74,10 +74,10 @@ func TestCommand_Matches(t *testing.T) { func TestCommander_Run_ErrorHandling(t *testing.T) { tests := []struct { - name string - handlerError error - expectedExit int - description string + name string + handlerError error + expectedExit int + description string }{ { name: "usage error", From a23f475c8d516d904bc48ebf8f27ec61656915c5 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc Date: Mon, 10 Nov 2025 23:55:25 -0700 Subject: [PATCH 9/9] Add test to ensure src snapshot upload --help prints its own usage text --- cmd/src/cmd_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/cmd/src/cmd_test.go b/cmd/src/cmd_test.go index d7181cea8c..f62323798e 100644 --- a/cmd/src/cmd_test.go +++ b/cmd/src/cmd_test.go @@ -3,6 +3,7 @@ package main import ( "bytes" "flag" + "fmt" "os" "os/exec" "testing" @@ -206,7 +207,60 @@ func TestCommander_Run_HelpFlag(t *testing.T) { } func TestCommander_Run_NestedHelpFlags(t *testing.T) { - t.Skip("Complex nested help flag testing requires integration with actual src commands") + if os.Getenv("TEST_SUBPROCESS") == "1" { + testHomeDir = os.Getenv("TEST_TEMP_DIR") + + uploadFlagSet := flag.NewFlagSet("upload", flag.ExitOnError) + uploadCmd := &command{ + flagSet: uploadFlagSet, + handler: func(args []string) error { return nil }, + usageFunc: func() { + fmt.Fprint(flag.CommandLine.Output(), "upload usage text") + }, + } + + snapshotCommands := commander{uploadCmd} + + snapshotFlagSet := flag.NewFlagSet("snapshot", flag.ExitOnError) + snapshotCmd := &command{ + flagSet: snapshotFlagSet, + handler: func(args []string) error { + snapshotCommands.run(snapshotFlagSet, "src snapshot", "snapshot usage text", args) + return nil + }, + usageFunc: func() { + fmt.Fprint(flag.CommandLine.Output(), "snapshot usage text") + }, + } + + cmdr := commander{snapshotCmd} + flagSet := flag.NewFlagSet("test", flag.ExitOnError) + args := []string{"snapshot", "upload", "--h"} + cmdr.run(flagSet, "src", "root usage", args) + return + } + + tempDir := t.TempDir() + cmd := exec.Command(os.Args[0], "-test.run=^TestCommander_Run_NestedHelpFlags$") + cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1", "TEST_TEMP_DIR="+tempDir) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() + + output := stdout.String() + stderr.String() + + if err != nil { + t.Errorf("expected success, got error: %v\noutput: %s", err, output) + } + + if !bytes.Contains([]byte(output), []byte("upload usage text")) { + t.Errorf("expected output to contain 'upload usage text', got:\n%s", output) + } + + if bytes.Contains([]byte(output), []byte("snapshot usage text")) { + t.Errorf("expected output NOT to contain 'snapshot usage text', got:\n%s", output) + } } func TestCommander_Run_InvalidSubcommand(t *testing.T) {