From 0f10a71a34d0a4427c1cdd0ee79418c88213f401 Mon Sep 17 00:00:00 2001 From: Jan Polasek Date: Mon, 22 Aug 2016 18:23:32 +0100 Subject: [PATCH] Unify the behaviour of error reporting when unknown subcommand is passed, add tests. --- bash_completions_test.go | 2 +- cobra_test.go | 100 +++++++++++++++++++++++++++++++-------- command.go | 12 ++--- 3 files changed, 87 insertions(+), 27 deletions(-) diff --git a/bash_completions_test.go b/bash_completions_test.go index 5f4cb4d1..d315c40a 100644 --- a/bash_completions_test.go +++ b/bash_completions_test.go @@ -53,7 +53,7 @@ COMPREPLY=( "hello" ) ) func TestBashCompletions(t *testing.T) { - c := initializeWithRootCmd() + c := initializeWithRootCmdWithRun() cmdEcho.AddCommand(cmdTimes) c.AddCommand(cmdEcho, cmdPrint, cmdDeprecated, cmdColon) diff --git a/cobra_test.go b/cobra_test.go index 881a621d..3eb1725b 100644 --- a/cobra_test.go +++ b/cobra_test.go @@ -27,6 +27,7 @@ var versionUsed int const strtwoParentHelp = "help message for parent flag strtwo" const strtwoChildHelp = "help message for child flag strtwo" +const nonExistentCommand = "non_existent_command" var cmdHidden = &Command{ Use: "hide [secret string to print]", @@ -212,7 +213,7 @@ func initializeWithSameName() *Command { return c } -func initializeWithRootCmd() *Command { +func initializeWithRootCmdWithRun() *Command { cmdRootWithRun.ResetCommands() tt, tp, te, tr, rootcalled = nil, nil, nil, nil, false flagInit() @@ -229,16 +230,28 @@ type resulter struct { } func fullSetupTest(input string) resulter { - c := initializeWithRootCmd() + c := initializeWithRootCmdWithRun() return fullTester(c, input) } func noRRSetupTestSilenced(input string) resulter { c := initialize() + + silenceErrors := c.SilenceErrors + silenceUsage := c.SilenceUsage + c.SilenceErrors = true c.SilenceUsage = true - return fullTester(c, input) + + res := fullTester(c, input) + + // Restore previous values of silencing errors and usage. + // This is a workaround for these values not being reset properly between tests. + c.SilenceErrors = silenceErrors + c.SilenceUsage = silenceUsage + + return res } func noRRSetupTest(input string) resulter { @@ -248,7 +261,7 @@ func noRRSetupTest(input string) resulter { } func rootOnlySetupTest(input string) resulter { - c := initializeWithRootCmd() + c := initializeWithRootCmdWithRun() return simpleTester(c, input) } @@ -585,7 +598,7 @@ func TestTrailingCommandFlags(t *testing.T) { } func TestInvalidSubcommandFlags(t *testing.T) { - cmd := initializeWithRootCmd() + cmd := initializeWithRootCmdWithRun() cmd.AddCommand(cmdTimes) result := simpleTester(cmd, "times --inttwo=2 --badflag=bar") @@ -599,7 +612,7 @@ func TestInvalidSubcommandFlags(t *testing.T) { } func TestSubcommandExecuteC(t *testing.T) { - cmd := initializeWithRootCmd() + cmd := initializeWithRootCmdWithRun() double := &Command{ Use: "double message", Run: func(c *Command, args []string) { @@ -634,7 +647,7 @@ func TestSubcommandExecuteC(t *testing.T) { } func TestSubcommandArgEvaluation(t *testing.T) { - cmd := initializeWithRootCmd() + cmd := initializeWithRootCmdWithRun() first := &Command{ Use: "first", @@ -753,7 +766,7 @@ func TestVisitParents(t *testing.T) { func TestRunnableRootCommandNilInput(t *testing.T) { var emptyArg []string - c := initializeWithRootCmd() + c := initializeWithRootCmdWithRun() buf := new(bytes.Buffer) // Testing flag with invalid input @@ -777,7 +790,7 @@ func TestRunnableRootCommandEmptyInput(t *testing.T) { args[0] = "" args[1] = "--introot=12" args[2] = "" - c := initializeWithRootCmd() + c := initializeWithRootCmdWithRun() buf := new(bytes.Buffer) // Testing flag with invalid input @@ -916,23 +929,70 @@ func TestRootUnknownCommand(t *testing.T) { func TestRootUnknownCommandSilenced(t *testing.T) { r := noRRSetupTestSilenced("bogus") - s := "Run 'cobra-test --help' for usage.\n" if r.Output != "" { - t.Errorf("Unexpected response.\nExpecting to be: \n\"\"\n Got:\n %q\n", s, r.Output) + t.Errorf("Unexpected response.\nExpecting to be: \n\"\"\n Got:\n %q\n", r.Output) } r = noRRSetupTestSilenced("--strtwo=a bogus") if r.Output != "" { - t.Errorf("Unexpected response.\nExpecting to be:\n\"\"\nGot:\n %q\n", s, r.Output) + t.Errorf("Unexpected response.\nExpecting to be:\n\"\"\nGot:\n %q\n", r.Output) } } -func TestRootSuggestions(t *testing.T) { - outputWithSuggestions := "Error: unknown command \"%s\" for \"cobra-test\"\n\nDid you mean this?\n\t%s\n\nRun 'cobra-test --help' for usage.\n" - outputWithoutSuggestions := "Error: unknown command \"%s\" for \"cobra-test\"\nRun 'cobra-test --help' for usage.\n" +func TestSubcommandChecksErrorsWhenSubcommandNotFound(t *testing.T) { + rootCmd := initialize() + rootCmd.AddCommand(cmdSubNoRun) + // Current semantics is that commands with no subcommands and no run print out the description. + cmdSubNoRun.AddCommand(cmdEcho) - cmd := initializeWithRootCmd() + result := simpleTester(rootCmd, nonExistentCommand) + if result.Error == nil { + t.Errorf("Unexpected response.\nExpected error, got success with output:\n %q\n", result.Output) + } + cmdName := rootCmd.Name() + expectedOutput := fmt.Sprintf("Error: unknown command %q for %q\nRun '%s --help' for usage.\n", nonExistentCommand, rootCmd.Name(), rootCmd.Name()) + if result.Output != expectedOutput { + t.Errorf("Unexpected response.\nExpected:\n %q\nGot:\n %q\n", expectedOutput, result.Output) + } + + result = simpleTester(rootCmd, cmdSubNoRun.Name() + " " + nonExistentCommand) + if result.Error == nil { + t.Errorf("Unexpected response.\nExpected error, got success with output:\n %q\n", result.Output) + } + cmdName = rootCmd.Name() + " " + cmdSubNoRun.Name() + expectedOutput = fmt.Sprintf("Error: unknown command %q for %q\nRun '%s --help' for usage.\n", nonExistentCommand, cmdName, cmdName) + if result.Output != expectedOutput { + t.Errorf("Unexpected response.\nExpected:\n %q\nGot:\n %q\n", expectedOutput, result.Output) + } +} + +func TestSubcommandChecksNoErrorsWhenRunDefined(t *testing.T) { + rootCmd := initializeWithRootCmdWithRun() + rootCmd.AddCommand(cmdEcho) + + result := simpleTester(rootCmd, nonExistentCommand) + if result.Error != nil { + t.Errorf("Unexpected response.\nExpected error, got success with output:\n %q\n", result.Output) + } + if !reflect.DeepEqual([]string{nonExistentCommand}, tr) { + t.Errorf("Unexpected response.\nExpected output:\n %q\nGot:\n %q\n", []string{nonExistentCommand}, tr) + } + + result = simpleTester(rootCmd, cmdEcho.Name() + " " + nonExistentCommand) + if result.Error != nil { + t.Errorf("Unexpected response.\nExpected error, got success with output:\n %q\n", result.Output) + } + if !reflect.DeepEqual([]string{nonExistentCommand}, te) { + t.Errorf("Unexpected response.\nExpected output:\n %q\nGot:\n %q\n", []string{nonExistentCommand}, te) + } +} + +func TestSubcommandSuggestions(t *testing.T) { + outputWithSuggestions := "Error: unknown command %q for \"cobra-test\"\n\nDid you mean this?\n\t%s\n\nRun 'cobra-test --help' for usage.\n" + outputWithoutSuggestions := "Error: unknown command %q for \"cobra-test\"\nRun 'cobra-test --help' for usage.\n" + + cmd := initialize() cmd.AddCommand(cmdTimes) tests := map[string]string{ @@ -1023,7 +1083,7 @@ func TestFlagsBeforeCommand(t *testing.T) { func TestRemoveCommand(t *testing.T) { versionUsed = 0 - c := initializeWithRootCmd() + c := initialize() c.AddCommand(cmdVersion1) c.RemoveCommand(cmdVersion1) x := fullTester(c, "version") @@ -1034,7 +1094,7 @@ func TestRemoveCommand(t *testing.T) { } func TestCommandWithoutSubcommands(t *testing.T) { - c := initializeWithRootCmd() + c := initializeWithRootCmdWithRun() x := simpleTester(c, "") if x.Error != nil { @@ -1044,7 +1104,7 @@ func TestCommandWithoutSubcommands(t *testing.T) { } func TestCommandWithoutSubcommandsWithArg(t *testing.T) { - c := initializeWithRootCmd() + c := initializeWithRootCmdWithRun() expectedArgs := []string{"arg"} x := simpleTester(c, "arg") @@ -1060,7 +1120,7 @@ func TestCommandWithoutSubcommandsWithArg(t *testing.T) { func TestReplaceCommandWithRemove(t *testing.T) { versionUsed = 0 - c := initializeWithRootCmd() + c := initializeWithRootCmdWithRun() c.AddCommand(cmdVersion1) c.RemoveCommand(cmdVersion1) c.AddCommand(cmdVersion2) diff --git a/command.go b/command.go index 083e4ea7..f8c39f9c 100644 --- a/command.go +++ b/command.go @@ -462,14 +462,14 @@ func (c *Command) Find(args []string) (*Command, []string, error) { return commandFound, a, nil } - // root command with subcommands, do subcommand checking - if commandFound == c && len(argsWOflags) > 0 { + // command with no run method, do subcommand checking + if commandFound.Run == nil && commandFound.RunE == nil && len(argsWOflags) > 0 { suggestionsString := "" - if !c.DisableSuggestions { - if c.SuggestionsMinimumDistance <= 0 { - c.SuggestionsMinimumDistance = 2 + if !commandFound.DisableSuggestions { + if commandFound.SuggestionsMinimumDistance <= 0 { + commandFound.SuggestionsMinimumDistance = 2 } - if suggestions := c.SuggestionsFor(argsWOflags[0]); len(suggestions) > 0 { + if suggestions := commandFound.SuggestionsFor(argsWOflags[0]); len(suggestions) > 0 { suggestionsString += "\n\nDid you mean this?\n" for _, s := range suggestions { suggestionsString += fmt.Sprintf("\t%v\n", s)