From 2cd7871821f9efac618dbb4a00606cdd51276ff8 Mon Sep 17 00:00:00 2001 From: umarcor Date: Mon, 18 Mar 2019 23:32:12 +0100 Subject: [PATCH 1/7] feat: generalize ValidArgs; use it implicitly with any validator --- args.go | 65 +++++------ args_test.go | 227 +++++++++++++++++++++++++++++++++++---- bash_completions_test.go | 2 +- command.go | 5 + user_guide.md | 40 ++++--- 5 files changed, 270 insertions(+), 69 deletions(-) diff --git a/args.go b/args.go index 20a022b3..aad963bd 100644 --- a/args.go +++ b/args.go @@ -7,6 +7,25 @@ import ( type PositionalArgs func(cmd *Command, args []string) error +// validateArgs returns an error if there are any positional args that are not in +// the `ValidArgs` field of `Command` +func validateArgs(cmd *Command, args []string) error { + if len(cmd.ValidArgs) > 0 { + // Remove any description that may be included in ValidArgs. + // A description is following a tab character. + var validArgs []string + for _, v := range cmd.ValidArgs { + validArgs = append(validArgs, strings.Split(v, "\t")[0]) + } + for _, v := range args { + if !stringInSlice(v, validArgs) { + return fmt.Errorf("invalid argument %q for %q%s", v, cmd.CommandPath(), cmd.findSuggestions(args[0])) + } + } + } + return nil +} + // Legacy arg validation has the following behaviour: // - root commands with no subcommands can take arbitrary arguments // - root commands with subcommands will do subcommand validity checking @@ -32,25 +51,6 @@ func NoArgs(cmd *Command, args []string) error { return nil } -// OnlyValidArgs returns an error if any args are not in the list of ValidArgs. -func OnlyValidArgs(cmd *Command, args []string) error { - if len(cmd.ValidArgs) > 0 { - // Remove any description that may be included in ValidArgs. - // A description is following a tab character. - var validArgs []string - for _, v := range cmd.ValidArgs { - validArgs = append(validArgs, strings.Split(v, "\t")[0]) - } - - for _, v := range args { - if !stringInSlice(v, validArgs) { - return fmt.Errorf("invalid argument %q for %q%s", v, cmd.CommandPath(), cmd.findSuggestions(args[0])) - } - } - } - return nil -} - // ArbitraryArgs never returns an error. func ArbitraryArgs(cmd *Command, args []string) error { return nil @@ -86,18 +86,6 @@ func ExactArgs(n int) PositionalArgs { } } -// ExactValidArgs returns an error if -// there are not exactly N positional args OR -// there are any positional args that are not in the `ValidArgs` field of `Command` -func ExactValidArgs(n int) PositionalArgs { - return func(cmd *Command, args []string) error { - if err := ExactArgs(n)(cmd, args); err != nil { - return err - } - return OnlyValidArgs(cmd, args) - } -} - // RangeArgs returns an error if the number of args is not within the expected range. func RangeArgs(min int, max int) PositionalArgs { return func(cmd *Command, args []string) error { @@ -119,3 +107,18 @@ func MatchAll(pargs ...PositionalArgs) PositionalArgs { return nil } } + +// ExactValidArgs returns an error if there are not exactly N positional args OR +// there are any positional args that are not in the `ValidArgs` field of `Command` +// +// Deprecated: now `ExactArgs` honors `ValidArgs`, when defined and not empty +func ExactValidArgs(n int) PositionalArgs { + return ExactArgs(n) +} + +// OnlyValidArgs returns an error if any args are not in the list of `ValidArgs`. +// +// Deprecated: now `ArbitraryArgs` honors `ValidArgs`, when defined and not empty +func OnlyValidArgs(cmd *Command, args []string) error { + return ArbitraryArgs(cmd, args) +} diff --git a/args_test.go b/args_test.go index 6bd41c8f..cffcf1d9 100644 --- a/args_test.go +++ b/args_test.go @@ -31,6 +31,7 @@ func validWithInvalidArgs(err error, t *testing.T) { if err == nil { t.Fatal("Expected an error") } + got := err.Error() expected := `invalid argument "a" for "c"` if got != expected { @@ -43,7 +44,7 @@ func noArgsWithArgs(err error, t *testing.T) { t.Fatal("Expected an error") } got := err.Error() - expected := `unknown command "illegal" for "c"` + expected := `unknown command "one" for "c"` if got != expected { t.Errorf("Expected: %q, got: %q", expected, got) } @@ -64,6 +65,7 @@ func maximumNArgsWithMoreArgs(err error, t *testing.T) { if err == nil { t.Fatal("Expected an error") } + got := err.Error() expected := "accepts at most 2 arg(s), received 3" if got != expected { @@ -93,6 +95,8 @@ func rangeArgsWithInvalidCount(err error, t *testing.T) { } } +// NoArgs + func TestNoArgs(t *testing.T) { c := getCommand(NoArgs, false) output, err := executeCommand(c) @@ -101,21 +105,17 @@ func TestNoArgs(t *testing.T) { func TestNoArgsWithArgs(t *testing.T) { c := getCommand(NoArgs, false) - _, err := executeCommand(c, "illegal") + _, err := executeCommand(c, "one") noArgsWithArgs(err, t) } -func TestOnlyValidArgs(t *testing.T) { - c := getCommand(OnlyValidArgs, true) - output, err := executeCommand(c, "one", "two") - expectSuccess(output, err, t) +func TestNoArgsWithArgsWithValid(t *testing.T) { + c := getCommand(NoArgs, true) + _, err := executeCommand(c, "one") + noArgsWithArgs(err, t) } -func TestOnlyValidArgsWithInvalidArgs(t *testing.T) { - c := getCommand(OnlyValidArgs, true) - _, err := executeCommand(c, "a") - validWithInvalidArgs(err, t) -} +// ArbitraryArgs func TestArbitraryArgs(t *testing.T) { c := getCommand(ArbitraryArgs, false) @@ -123,72 +123,172 @@ func TestArbitraryArgs(t *testing.T) { expectSuccess(output, err, t) } +func TestArbitraryArgsWithValid(t *testing.T) { + c := getCommand(ArbitraryArgs, true) + output, err := executeCommand(c, "one", "two") + expectSuccess(output, err, t) +} + +func TestArbitraryArgsWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(ArbitraryArgs, true) + _, err := executeCommand(c, "a") + validWithInvalidArgs(err, t) +} + +// MinimumNArgs + func TestMinimumNArgs(t *testing.T) { c := getCommand(MinimumNArgs(2), false) output, err := executeCommand(c, "a", "b", "c") expectSuccess(output, err, t) } +func TestMinimumNArgsWithValid(t *testing.T) { + c := getCommand(MinimumNArgs(2), true) + output, err := executeCommand(c, "one", "three") + expectSuccess(output, err, t) +} + +func TestMinimumNArgsWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(MinimumNArgs(2), true) + _, err := executeCommand(c, "a", "b") + validWithInvalidArgs(err, t) +} + func TestMinimumNArgsWithLessArgs(t *testing.T) { c := getCommand(MinimumNArgs(2), false) _, err := executeCommand(c, "a") minimumNArgsWithLessArgs(err, t) } +func TestMinimumNArgsWithLessArgsWithValid(t *testing.T) { + c := getCommand(MinimumNArgs(2), true) + _, err := executeCommand(c, "one") + minimumNArgsWithLessArgs(err, t) +} + +func TestMinimumNArgsWithLessArgsWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(MinimumNArgs(2), true) + _, err := executeCommand(c, "a") + validWithInvalidArgs(err, t) +} + +// MaximumNArgs + func TestMaximumNArgs(t *testing.T) { c := getCommand(MaximumNArgs(3), false) output, err := executeCommand(c, "a", "b") expectSuccess(output, err, t) } +func TestMaximumNArgsWithValid(t *testing.T) { + c := getCommand(MaximumNArgs(2), true) + output, err := executeCommand(c, "one", "three") + expectSuccess(output, err, t) +} + +func TestMaximumNArgsWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(MaximumNArgs(2), true) + _, err := executeCommand(c, "a", "b") + validWithInvalidArgs(err, t) +} + func TestMaximumNArgsWithMoreArgs(t *testing.T) { c := getCommand(MaximumNArgs(2), false) _, err := executeCommand(c, "a", "b", "c") maximumNArgsWithMoreArgs(err, t) } +func TestMaximumNArgsWithMoreArgsWithValid(t *testing.T) { + c := getCommand(MaximumNArgs(2), true) + _, err := executeCommand(c, "one", "three", "two") + maximumNArgsWithMoreArgs(err, t) +} + +func TestMaximumNArgsWithMoreArgsWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(MaximumNArgs(2), true) + _, err := executeCommand(c, "a", "b", "c") + validWithInvalidArgs(err, t) +} + +// ExactArgs + func TestExactArgs(t *testing.T) { c := getCommand(ExactArgs(3), false) output, err := executeCommand(c, "a", "b", "c") expectSuccess(output, err, t) } +func TestExactArgsWithValid(t *testing.T) { + c := getCommand(ExactArgs(3), true) + output, err := executeCommand(c, "three", "one", "two") + expectSuccess(output, err, t) +} + +func TestExactArgsWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(ExactArgs(3), true) + _, err := executeCommand(c, "three", "a", "two") + validWithInvalidArgs(err, t) +} + func TestExactArgsWithInvalidCount(t *testing.T) { c := getCommand(ExactArgs(2), false) _, err := executeCommand(c, "a", "b", "c") exactArgsWithInvalidCount(err, t) } -func TestExactValidArgs(t *testing.T) { - c := getCommand(ExactValidArgs(3), true) - output, err := executeCommand(c, "three", "one", "two") - expectSuccess(output, err, t) -} - -func TestExactValidArgsWithInvalidCount(t *testing.T) { - c := getCommand(ExactValidArgs(2), false) +func TestExactArgsWithInvalidCountWithValid(t *testing.T) { + c := getCommand(ExactArgs(2), true) _, err := executeCommand(c, "three", "one", "two") exactArgsWithInvalidCount(err, t) } -func TestExactValidArgsWithInvalidArgs(t *testing.T) { - c := getCommand(ExactValidArgs(3), true) +func TestExactArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(ExactArgs(2), true) _, err := executeCommand(c, "three", "a", "two") validWithInvalidArgs(err, t) } +// RangeArgs + func TestRangeArgs(t *testing.T) { c := getCommand(RangeArgs(2, 4), false) output, err := executeCommand(c, "a", "b", "c") expectSuccess(output, err, t) } +func TestRangeArgsWithValid(t *testing.T) { + c := getCommand(RangeArgs(2, 4), true) + output, err := executeCommand(c, "three", "one", "two") + expectSuccess(output, err, t) +} + +func TestRangeArgsWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(RangeArgs(2, 4), true) + _, err := executeCommand(c, "three", "a", "two") + validWithInvalidArgs(err, t) +} + func TestRangeArgsWithInvalidCount(t *testing.T) { c := getCommand(RangeArgs(2, 4), false) _, err := executeCommand(c, "a") rangeArgsWithInvalidCount(err, t) } +func TestRangeArgsWithInvalidCountWithValid(t *testing.T) { + c := getCommand(RangeArgs(2, 4), true) + _, err := executeCommand(c, "two") + rangeArgsWithInvalidCount(err, t) +} + +func TestRangeArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) { + c := getCommand(RangeArgs(2, 4), true) + _, err := executeCommand(c, "a") + validWithInvalidArgs(err, t) +} + +// Takes(No)Args + func TestRootTakesNoArgs(t *testing.T) { rootCmd := &Command{Use: "root", Run: emptyRun} childCmd := &Command{Use: "child", Run: emptyRun} @@ -293,6 +393,91 @@ func TestMatchAll(t *testing.T) { } } +// DEPRECATED + +func TestOnlyValidArgs(t *testing.T) { + c := &Command{ + Use: "c", + Args: OnlyValidArgs, + ValidArgs: []string{"one", "two"}, + Run: emptyRun, + } + + output, err := executeCommand(c, "one", "two") + if output != "" { + t.Errorf("Unexpected output: %v", output) + } + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } +} + +func TestOnlyValidArgsWithInvalidArgs(t *testing.T) { + c := &Command{ + Use: "c", + Args: OnlyValidArgs, + ValidArgs: []string{"one", "two"}, + Run: emptyRun, + } + + _, err := executeCommand(c, "three") + if err == nil { + t.Fatal("Expected an error") + } + + got := err.Error() + expected := `invalid argument "three" for "c"` + if got != expected { + t.Errorf("Expected: %q, got: %q", expected, got) + } +} + +func TestExactValidArgs(t *testing.T) { + c := &Command{Use: "c", Args: ExactValidArgs(3), ValidArgs: []string{"a", "b", "c"}, Run: emptyRun} + output, err := executeCommand(c, "a", "b", "c") + if output != "" { + t.Errorf("Unexpected output: %v", output) + } + if err != nil { + t.Errorf("Unexpected error: %v", err) + } +} + +func TestExactValidArgsWithInvalidCount(t *testing.T) { + c := &Command{Use: "c", Args: ExactValidArgs(2), Run: emptyRun} + _, err := executeCommand(c, "a", "b", "c") + + if err == nil { + t.Fatal("Expected an error") + } + + got := err.Error() + expected := "accepts 2 arg(s), received 3" + if got != expected { + t.Fatalf("Expected %q, got %q", expected, got) + } +} + +func TestExactValidArgsWithInvalidArgs(t *testing.T) { + c := &Command{ + Use: "c", + Args: ExactValidArgs(1), + ValidArgs: []string{"one", "two"}, + Run: emptyRun, + } + + _, err := executeCommand(c, "three") + if err == nil { + t.Fatal("Expected an error") + } + + got := err.Error() + expected := `invalid argument "three" for "c"` + if got != expected { + t.Errorf("Expected: %q, got: %q", expected, got) + } +} + // This test make sure we keep backwards-compatibility with respect // to the legacyArgs() function. // It makes sure the root command accepts arguments if it does not have diff --git a/bash_completions_test.go b/bash_completions_test.go index 6896e91c..a383b007 100644 --- a/bash_completions_test.go +++ b/bash_completions_test.go @@ -139,7 +139,7 @@ func TestBashCompletions(t *testing.T) { timesCmd := &Command{ Use: "times [# times] [string to echo]", SuggestFor: []string{"counts"}, - Args: OnlyValidArgs, + Args: ArbitraryArgs, ValidArgs: []string{"one", "two", "three", "four"}, Short: "Echo anything to the screen more times", Long: "a slightly useless command for testing.", diff --git a/command.go b/command.go index 675bb134..5befacae 100644 --- a/command.go +++ b/command.go @@ -1011,10 +1011,15 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, err } +// ValidateArgs returns an error if any positional args are not in +// the `ValidArgs` field of `Command` func (c *Command) ValidateArgs(args []string) error { if c.Args == nil { return ArbitraryArgs(c, args) } + if err := validateArgs(c, args); err != nil { + return err + } return c.Args(c, args) } diff --git a/user_guide.md b/user_guide.md index 5a7acf88..99c6c31d 100644 --- a/user_guide.md +++ b/user_guide.md @@ -302,15 +302,15 @@ rootCmd.MarkPersistentFlagRequired("region") ### Flag Groups -If you have different flags that must be provided together (e.g. if they provide the `--username` flag they MUST provide the `--password` flag as well) then +If you have different flags that must be provided together (e.g. if they provide the `--username` flag they MUST provide the `--password` flag as well) then Cobra can enforce that requirement: ```go rootCmd.Flags().StringVarP(&u, "username", "u", "", "Username (required if password is set)") rootCmd.Flags().StringVarP(&pw, "password", "p", "", "Password (required if username is set)") rootCmd.MarkFlagsRequiredTogether("username", "password") -``` +``` -You can also prevent different flags from being provided together if they represent mutually +You can also prevent different flags from being provided together if they represent mutually exclusive options such as specifying an output format as either `--json` or `--yaml` but never both: ```go rootCmd.Flags().BoolVar(&u, "json", false, "Output in JSON") @@ -327,29 +327,37 @@ In both of these cases: ## Positional and Custom Arguments Validation of positional arguments can be specified using the `Args` field of `Command`. -If `Args` is undefined or `nil`, it defaults to `ArbitraryArgs`. - The following validators are built in: -- `NoArgs` - the command will report an error if there are any positional args. -- `ArbitraryArgs` - the command will accept any args. -- `OnlyValidArgs` - the command will report an error if there are any positional args that are not in the `ValidArgs` field of `Command`. -- `MinimumNArgs(int)` - the command will report an error if there are not at least N positional args. -- `MaximumNArgs(int)` - the command will report an error if there are more than N positional args. -- `ExactArgs(int)` - the command will report an error if there are not exactly N positional args. -- `ExactValidArgs(int)` - the command will report an error if there are not exactly N positional args OR if there are any positional args that are not in the `ValidArgs` field of `Command` -- `RangeArgs(min, max)` - the command will report an error if the number of args is not between the minimum and maximum number of expected args. +- `NoArgs` - report an error if there are any positional args. +- `ArbitraryArgs` - accept any number of args. +- `MinimumNArgs(int)` - report an error if less than N positional args are provided. +- `MaximumNArgs(int)` - report an error if more than N positional args are provided. +- `ExactArgs(int)` - report an error if there are not exactly N positional args. +- `RangeArgs(min, max)` - report an error if the number of args is not between `min` and `max`. - `MatchAll(pargs ...PositionalArgs)` - enables combining existing checks with arbitrary other checks (e.g. you want to check the ExactArgs length along with other qualities). -An example of setting the custom validator: +If `Args` is undefined or `nil`, it defaults to `ArbitraryArgs`. + +Field `ValidArgs` of type `[]string` can be defined in `Command`, in order to report an error if there are any +positional args that are not in the list. +This validation is executed implicitly before the validator defined in `Args`. + +> NOTE: `OnlyValidArgs` and `ExactValidArgs(int)` are now deprecated. +> `ArbitraryArgs` and `ExactArgs(int)` provide the same functionality now. + +Moreover, it is possible to set any custom validator that satisfies `func(cmd *cobra.Command, args []string) error`. +For example: ```go var cmd = &cobra.Command{ Short: "hello", Args: func(cmd *cobra.Command, args []string) error { - if len(args) < 1 { - return errors.New("requires a color argument") + // Optionally run one of the validators provided by cobra + if err := cobra.MinimumNArgs(1)(cmd, args); err != nil { + return err } + // Run the custom validation logic if myapp.IsValidColor(args[0]) { return nil } From 78048ea8113eb55a8b3938b7539a8fd911ac3976 Mon Sep 17 00:00:00 2001 From: umarcor Date: Mon, 1 Feb 2021 11:29:39 +0100 Subject: [PATCH 2/7] command/ValidateArgs: check ValidArgs regardless of Args being nil --- command.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/command.go b/command.go index 5befacae..c12c6c7c 100644 --- a/command.go +++ b/command.go @@ -1011,15 +1011,16 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, err } -// ValidateArgs returns an error if any positional args are not in -// the `ValidArgs` field of `Command` +// ValidateArgs returns an error if any positional args are not in the +// `ValidArgs` field of `Command`. Then, run the `Args` validator, if +// specified. func (c *Command) ValidateArgs(args []string) error { - if c.Args == nil { - return ArbitraryArgs(c, args) - } if err := validateArgs(c, args); err != nil { return err } + if c.Args == nil { + return ArbitraryArgs(c, args) + } return c.Args(c, args) } From d4d0679f4b2d4b1afbd01f8e7a07cf279ea8968b Mon Sep 17 00:00:00 2001 From: umarcor Date: Tue, 19 Mar 2019 00:45:32 +0100 Subject: [PATCH 3/7] add expectError helper function to args_test --- args_test.go | 111 ++++++++++++++++----------------------------------- 1 file changed, 34 insertions(+), 77 deletions(-) diff --git a/args_test.go b/args_test.go index cffcf1d9..6fdacc27 100644 --- a/args_test.go +++ b/args_test.go @@ -27,74 +27,31 @@ func expectSuccess(output string, err error, t *testing.T) { } } -func validWithInvalidArgs(err error, t *testing.T) { +func expectError(err error, t *testing.T, ex string) { if err == nil { t.Fatal("Expected an error") } - got := err.Error() + expected := `invalid argument "a" for "c"` + switch ex { + case "no": + expected = `unknown command "one" for "c"` + case "min": + expected = "requires at least 2 arg(s), only received 1" + case "max": + expected = "accepts at most 2 arg(s), received 3" + case "exact": + expected = "accepts 2 arg(s), received 3" + case "range": + expected = "accepts between 2 and 4 arg(s), received 1" + } + if got != expected { t.Errorf("Expected: %q, got: %q", expected, got) } } -func noArgsWithArgs(err error, t *testing.T) { - if err == nil { - t.Fatal("Expected an error") - } - got := err.Error() - expected := `unknown command "one" for "c"` - if got != expected { - t.Errorf("Expected: %q, got: %q", expected, got) - } -} - -func minimumNArgsWithLessArgs(err error, t *testing.T) { - if err == nil { - t.Fatal("Expected an error") - } - got := err.Error() - expected := "requires at least 2 arg(s), only received 1" - if got != expected { - t.Fatalf("Expected %q, got %q", expected, got) - } -} - -func maximumNArgsWithMoreArgs(err error, t *testing.T) { - if err == nil { - t.Fatal("Expected an error") - } - - got := err.Error() - expected := "accepts at most 2 arg(s), received 3" - if got != expected { - t.Fatalf("Expected %q, got %q", expected, got) - } -} - -func exactArgsWithInvalidCount(err error, t *testing.T) { - if err == nil { - t.Fatal("Expected an error") - } - got := err.Error() - expected := "accepts 2 arg(s), received 3" - if got != expected { - t.Fatalf("Expected %q, got %q", expected, got) - } -} - -func rangeArgsWithInvalidCount(err error, t *testing.T) { - if err == nil { - t.Fatal("Expected an error") - } - got := err.Error() - expected := "accepts between 2 and 4 arg(s), received 1" - if got != expected { - t.Fatalf("Expected %q, got %q", expected, got) - } -} - // NoArgs func TestNoArgs(t *testing.T) { @@ -106,13 +63,13 @@ func TestNoArgs(t *testing.T) { func TestNoArgsWithArgs(t *testing.T) { c := getCommand(NoArgs, false) _, err := executeCommand(c, "one") - noArgsWithArgs(err, t) + expectError(err, t, "no") } func TestNoArgsWithArgsWithValid(t *testing.T) { c := getCommand(NoArgs, true) _, err := executeCommand(c, "one") - noArgsWithArgs(err, t) + expectError(err, t, "no") } // ArbitraryArgs @@ -132,7 +89,7 @@ func TestArbitraryArgsWithValid(t *testing.T) { func TestArbitraryArgsWithValidWithInvalidArgs(t *testing.T) { c := getCommand(ArbitraryArgs, true) _, err := executeCommand(c, "a") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } // MinimumNArgs @@ -152,25 +109,25 @@ func TestMinimumNArgsWithValid(t *testing.T) { func TestMinimumNArgsWithValidWithInvalidArgs(t *testing.T) { c := getCommand(MinimumNArgs(2), true) _, err := executeCommand(c, "a", "b") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } func TestMinimumNArgsWithLessArgs(t *testing.T) { c := getCommand(MinimumNArgs(2), false) _, err := executeCommand(c, "a") - minimumNArgsWithLessArgs(err, t) + expectError(err, t, "min") } func TestMinimumNArgsWithLessArgsWithValid(t *testing.T) { c := getCommand(MinimumNArgs(2), true) _, err := executeCommand(c, "one") - minimumNArgsWithLessArgs(err, t) + expectError(err, t, "min") } func TestMinimumNArgsWithLessArgsWithValidWithInvalidArgs(t *testing.T) { c := getCommand(MinimumNArgs(2), true) _, err := executeCommand(c, "a") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } // MaximumNArgs @@ -190,25 +147,25 @@ func TestMaximumNArgsWithValid(t *testing.T) { func TestMaximumNArgsWithValidWithInvalidArgs(t *testing.T) { c := getCommand(MaximumNArgs(2), true) _, err := executeCommand(c, "a", "b") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } func TestMaximumNArgsWithMoreArgs(t *testing.T) { c := getCommand(MaximumNArgs(2), false) _, err := executeCommand(c, "a", "b", "c") - maximumNArgsWithMoreArgs(err, t) + expectError(err, t, "max") } func TestMaximumNArgsWithMoreArgsWithValid(t *testing.T) { c := getCommand(MaximumNArgs(2), true) _, err := executeCommand(c, "one", "three", "two") - maximumNArgsWithMoreArgs(err, t) + expectError(err, t, "max") } func TestMaximumNArgsWithMoreArgsWithValidWithInvalidArgs(t *testing.T) { c := getCommand(MaximumNArgs(2), true) _, err := executeCommand(c, "a", "b", "c") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } // ExactArgs @@ -228,25 +185,25 @@ func TestExactArgsWithValid(t *testing.T) { func TestExactArgsWithValidWithInvalidArgs(t *testing.T) { c := getCommand(ExactArgs(3), true) _, err := executeCommand(c, "three", "a", "two") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } func TestExactArgsWithInvalidCount(t *testing.T) { c := getCommand(ExactArgs(2), false) _, err := executeCommand(c, "a", "b", "c") - exactArgsWithInvalidCount(err, t) + expectError(err, t, "exact") } func TestExactArgsWithInvalidCountWithValid(t *testing.T) { c := getCommand(ExactArgs(2), true) _, err := executeCommand(c, "three", "one", "two") - exactArgsWithInvalidCount(err, t) + expectError(err, t, "exact") } func TestExactArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) { c := getCommand(ExactArgs(2), true) _, err := executeCommand(c, "three", "a", "two") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } // RangeArgs @@ -266,25 +223,25 @@ func TestRangeArgsWithValid(t *testing.T) { func TestRangeArgsWithValidWithInvalidArgs(t *testing.T) { c := getCommand(RangeArgs(2, 4), true) _, err := executeCommand(c, "three", "a", "two") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } func TestRangeArgsWithInvalidCount(t *testing.T) { c := getCommand(RangeArgs(2, 4), false) _, err := executeCommand(c, "a") - rangeArgsWithInvalidCount(err, t) + expectError(err, t, "range") } func TestRangeArgsWithInvalidCountWithValid(t *testing.T) { c := getCommand(RangeArgs(2, 4), true) _, err := executeCommand(c, "two") - rangeArgsWithInvalidCount(err, t) + expectError(err, t, "range") } func TestRangeArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) { c := getCommand(RangeArgs(2, 4), true) _, err := executeCommand(c, "a") - validWithInvalidArgs(err, t) + expectError(err, t, "valid") } // Takes(No)Args From d507a44e7ac16f03797f05e2deb9ab46997ea547 Mon Sep 17 00:00:00 2001 From: umarcor Date: Tue, 19 Mar 2019 20:30:05 +0100 Subject: [PATCH 4/7] args_test: style --- args_test.go | 317 ++++++++++++++++++--------------------------------- 1 file changed, 112 insertions(+), 205 deletions(-) diff --git a/args_test.go b/args_test.go index 6fdacc27..4c1fd42a 100644 --- a/args_test.go +++ b/args_test.go @@ -6,7 +6,7 @@ import ( "testing" ) -func getCommand(args PositionalArgs, withValid bool) *Command { +func newCommand(args PositionalArgs, withValid bool) *Command { c := &Command{ Use: "c", Args: args, @@ -31,23 +31,15 @@ func expectError(err error, t *testing.T, ex string) { if err == nil { t.Fatal("Expected an error") } - got := err.Error() - - expected := `invalid argument "a" for "c"` - switch ex { - case "no": - expected = `unknown command "one" for "c"` - case "min": - expected = "requires at least 2 arg(s), only received 1" - case "max": - expected = "accepts at most 2 arg(s), received 3" - case "exact": - expected = "accepts 2 arg(s), received 3" - case "range": - expected = "accepts between 2 and 4 arg(s), received 1" - } - - if got != expected { + expected := map[string]string{ + "valid": `invalid argument "a" for "c"`, + "no": `unknown command "one" for "c"`, + "min": "requires at least 2 arg(s), only received 1", + "max": "accepts at most 2 arg(s), received 3", + "exact": "accepts 2 arg(s), received 3", + "range": "accepts between 2 and 4 arg(s), received 1", + }[ex] + if got := err.Error(); got != expected { t.Errorf("Expected: %q, got: %q", expected, got) } } @@ -55,193 +47,163 @@ func expectError(err error, t *testing.T, ex string) { // NoArgs func TestNoArgs(t *testing.T) { - c := getCommand(NoArgs, false) - output, err := executeCommand(c) - expectSuccess(output, err, t) + o, e := executeCommand(newCommand(NoArgs, false)) + expectSuccess(o, e, t) } -func TestNoArgsWithArgs(t *testing.T) { - c := getCommand(NoArgs, false) - _, err := executeCommand(c, "one") - expectError(err, t, "no") +func TestNoArgs_WithArgs(t *testing.T) { + _, e := executeCommand(newCommand(NoArgs, false), "one") + expectError(e, t, "no") } -func TestNoArgsWithArgsWithValid(t *testing.T) { - c := getCommand(NoArgs, true) - _, err := executeCommand(c, "one") - expectError(err, t, "no") +func TestNoArgs_WithArgs_WithValid(t *testing.T) { + _, e := executeCommand(newCommand(NoArgs, true), "one") + expectError(e, t, "no") } // ArbitraryArgs func TestArbitraryArgs(t *testing.T) { - c := getCommand(ArbitraryArgs, false) - output, err := executeCommand(c, "a", "b") - expectSuccess(output, err, t) + o, e := executeCommand(newCommand(ArbitraryArgs, false), "a", "b") + expectSuccess(o, e, t) } -func TestArbitraryArgsWithValid(t *testing.T) { - c := getCommand(ArbitraryArgs, true) - output, err := executeCommand(c, "one", "two") - expectSuccess(output, err, t) +func TestArbitraryArgs_WithValid(t *testing.T) { + o, e := executeCommand(newCommand(ArbitraryArgs, true), "one", "two") + expectSuccess(o, e, t) } -func TestArbitraryArgsWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(ArbitraryArgs, true) - _, err := executeCommand(c, "a") - expectError(err, t, "valid") +func TestArbitraryArgs_WithValid_WithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(ArbitraryArgs, true), "a") + expectError(e, t, "valid") } // MinimumNArgs func TestMinimumNArgs(t *testing.T) { - c := getCommand(MinimumNArgs(2), false) - output, err := executeCommand(c, "a", "b", "c") - expectSuccess(output, err, t) + o, e := executeCommand(newCommand(MinimumNArgs(2), false), "a", "b", "c") + expectSuccess(o, e, t) } -func TestMinimumNArgsWithValid(t *testing.T) { - c := getCommand(MinimumNArgs(2), true) - output, err := executeCommand(c, "one", "three") - expectSuccess(output, err, t) +func TestMinimumNArgs_WithValid(t *testing.T) { + o, e := executeCommand(newCommand(MinimumNArgs(2), true), "one", "three") + expectSuccess(o, e, t) } -func TestMinimumNArgsWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(MinimumNArgs(2), true) - _, err := executeCommand(c, "a", "b") - expectError(err, t, "valid") +func TestMinimumNArgs_WithValid_WithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(MinimumNArgs(2), true), "a", "b") + expectError(e, t, "valid") } -func TestMinimumNArgsWithLessArgs(t *testing.T) { - c := getCommand(MinimumNArgs(2), false) - _, err := executeCommand(c, "a") - expectError(err, t, "min") +func TestMinimumNArgs_WithLessArgs(t *testing.T) { + _, e := executeCommand(newCommand(MinimumNArgs(2), false), "a") + expectError(e, t, "min") } -func TestMinimumNArgsWithLessArgsWithValid(t *testing.T) { - c := getCommand(MinimumNArgs(2), true) - _, err := executeCommand(c, "one") - expectError(err, t, "min") +func TestMinimumNArgs_WithValid_WithLessArgs(t *testing.T) { + _, e := executeCommand(newCommand(MinimumNArgs(2), true), "one") + expectError(e, t, "min") } -func TestMinimumNArgsWithLessArgsWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(MinimumNArgs(2), true) - _, err := executeCommand(c, "a") - expectError(err, t, "valid") +func TestMinimumNArgs_WithValid_WithLessArgsWithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(MinimumNArgs(2), true), "a") + expectError(e, t, "valid") } // MaximumNArgs func TestMaximumNArgs(t *testing.T) { - c := getCommand(MaximumNArgs(3), false) - output, err := executeCommand(c, "a", "b") - expectSuccess(output, err, t) + o, e := executeCommand(newCommand(MaximumNArgs(3), false), "a", "b") + expectSuccess(o, e, t) } -func TestMaximumNArgsWithValid(t *testing.T) { - c := getCommand(MaximumNArgs(2), true) - output, err := executeCommand(c, "one", "three") - expectSuccess(output, err, t) +func TestMaximumNArgs_WithValid(t *testing.T) { + o, e := executeCommand(newCommand(MaximumNArgs(2), true), "one", "three") + expectSuccess(o, e, t) } -func TestMaximumNArgsWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(MaximumNArgs(2), true) - _, err := executeCommand(c, "a", "b") - expectError(err, t, "valid") +func TestMaximumNArgs_WithValid_WithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(MaximumNArgs(2), true), "a", "b") + expectError(e, t, "valid") } -func TestMaximumNArgsWithMoreArgs(t *testing.T) { - c := getCommand(MaximumNArgs(2), false) - _, err := executeCommand(c, "a", "b", "c") - expectError(err, t, "max") +func TestMaximumNArgs_WithMoreArgs(t *testing.T) { + _, e := executeCommand(newCommand(MaximumNArgs(2), false), "a", "b", "c") + expectError(e, t, "max") } -func TestMaximumNArgsWithMoreArgsWithValid(t *testing.T) { - c := getCommand(MaximumNArgs(2), true) - _, err := executeCommand(c, "one", "three", "two") - expectError(err, t, "max") +func TestMaximumNArgs_WithValid_WithMoreArgs(t *testing.T) { + _, e := executeCommand(newCommand(MaximumNArgs(2), true), "one", "three", "two") + expectError(e, t, "max") } -func TestMaximumNArgsWithMoreArgsWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(MaximumNArgs(2), true) - _, err := executeCommand(c, "a", "b", "c") - expectError(err, t, "valid") +func TestMaximumNArgs_WithValid_WithMoreArgsWithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(MaximumNArgs(2), true), "a", "b", "c") + expectError(e, t, "valid") } // ExactArgs func TestExactArgs(t *testing.T) { - c := getCommand(ExactArgs(3), false) - output, err := executeCommand(c, "a", "b", "c") - expectSuccess(output, err, t) + o, e := executeCommand(newCommand(ExactArgs(3), false), "a", "b", "c") + expectSuccess(o, e, t) } -func TestExactArgsWithValid(t *testing.T) { - c := getCommand(ExactArgs(3), true) - output, err := executeCommand(c, "three", "one", "two") - expectSuccess(output, err, t) +func TestExactArgs_WithValid(t *testing.T) { + o, e := executeCommand(newCommand(ExactArgs(3), true), "three", "one", "two") + expectSuccess(o, e, t) } -func TestExactArgsWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(ExactArgs(3), true) - _, err := executeCommand(c, "three", "a", "two") - expectError(err, t, "valid") +func TestExactArgs_WithValid_WithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(ExactArgs(3), true), "three", "a", "two") + expectError(e, t, "valid") } -func TestExactArgsWithInvalidCount(t *testing.T) { - c := getCommand(ExactArgs(2), false) - _, err := executeCommand(c, "a", "b", "c") - expectError(err, t, "exact") +func TestExactArgs_WithInvalidCount(t *testing.T) { + _, e := executeCommand(newCommand(ExactArgs(2), false), "a", "b", "c") + expectError(e, t, "exact") } -func TestExactArgsWithInvalidCountWithValid(t *testing.T) { - c := getCommand(ExactArgs(2), true) - _, err := executeCommand(c, "three", "one", "two") - expectError(err, t, "exact") +func TestExactArgs_WithValid_WithInvalidCount(t *testing.T) { + _, e := executeCommand(newCommand(ExactArgs(2), true), "three", "one", "two") + expectError(e, t, "exact") } -func TestExactArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(ExactArgs(2), true) - _, err := executeCommand(c, "three", "a", "two") - expectError(err, t, "valid") +func TestExactArgs_WithValid_WithInvalidCountWithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(ExactArgs(2), true), "three", "a", "two") + expectError(e, t, "valid") } // RangeArgs func TestRangeArgs(t *testing.T) { - c := getCommand(RangeArgs(2, 4), false) - output, err := executeCommand(c, "a", "b", "c") - expectSuccess(output, err, t) + o, e := executeCommand(newCommand(RangeArgs(2, 4), false), "a", "b", "c") + expectSuccess(o, e, t) } -func TestRangeArgsWithValid(t *testing.T) { - c := getCommand(RangeArgs(2, 4), true) - output, err := executeCommand(c, "three", "one", "two") - expectSuccess(output, err, t) +func TestRangeArgs_WithValid(t *testing.T) { + o, e := executeCommand(newCommand(RangeArgs(2, 4), true), "three", "one", "two") + expectSuccess(o, e, t) } -func TestRangeArgsWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(RangeArgs(2, 4), true) - _, err := executeCommand(c, "three", "a", "two") - expectError(err, t, "valid") +func TestRangeArgs_WithValid_WithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "three", "a", "two") + expectError(e, t, "valid") } -func TestRangeArgsWithInvalidCount(t *testing.T) { - c := getCommand(RangeArgs(2, 4), false) - _, err := executeCommand(c, "a") - expectError(err, t, "range") +func TestRangeArgs_WithInvalidCount(t *testing.T) { + _, e := executeCommand(newCommand(RangeArgs(2, 4), false), "a") + expectError(e, t, "range") } -func TestRangeArgsWithInvalidCountWithValid(t *testing.T) { - c := getCommand(RangeArgs(2, 4), true) - _, err := executeCommand(c, "two") - expectError(err, t, "range") +func TestRangeArgs_WithValid_WithInvalidCount(t *testing.T) { + _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "two") + expectError(e, t, "range") } -func TestRangeArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) { - c := getCommand(RangeArgs(2, 4), true) - _, err := executeCommand(c, "a") - expectError(err, t, "valid") +func TestRangeArgs_WithValid_WithInvalidCountWithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "a") + expectError(e, t, "valid") } // Takes(No)Args @@ -352,87 +314,32 @@ func TestMatchAll(t *testing.T) { // DEPRECATED -func TestOnlyValidArgs(t *testing.T) { - c := &Command{ - Use: "c", - Args: OnlyValidArgs, - ValidArgs: []string{"one", "two"}, - Run: emptyRun, - } - - output, err := executeCommand(c, "one", "two") - if output != "" { - t.Errorf("Unexpected output: %v", output) - } - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } +func TestDEPRECATED_OnlyValidArgs(t *testing.T) { + o, e := executeCommand(newCommand(OnlyValidArgs, true), "one", "two") + expectSuccess(o, e, t) } -func TestOnlyValidArgsWithInvalidArgs(t *testing.T) { - c := &Command{ - Use: "c", - Args: OnlyValidArgs, - ValidArgs: []string{"one", "two"}, - Run: emptyRun, - } - - _, err := executeCommand(c, "three") - if err == nil { - t.Fatal("Expected an error") - } - - got := err.Error() - expected := `invalid argument "three" for "c"` - if got != expected { - t.Errorf("Expected: %q, got: %q", expected, got) - } +func TestDEPRECATED_OnlyValidArgs_WithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(OnlyValidArgs, true), "a") + expectError(e, t, "valid") } -func TestExactValidArgs(t *testing.T) { - c := &Command{Use: "c", Args: ExactValidArgs(3), ValidArgs: []string{"a", "b", "c"}, Run: emptyRun} - output, err := executeCommand(c, "a", "b", "c") - if output != "" { - t.Errorf("Unexpected output: %v", output) - } - if err != nil { - t.Errorf("Unexpected error: %v", err) - } +func TestDEPRECATED_ExactValidArgs(t *testing.T) { + // Note that the order is not required to be the same: + // Definition: "one", "two", "three" + // Execution: "two", "three", "one" + o, e := executeCommand(newCommand(ExactValidArgs(3), true), "two", "three", "one") + expectSuccess(o, e, t) } -func TestExactValidArgsWithInvalidCount(t *testing.T) { - c := &Command{Use: "c", Args: ExactValidArgs(2), Run: emptyRun} - _, err := executeCommand(c, "a", "b", "c") - - if err == nil { - t.Fatal("Expected an error") - } - - got := err.Error() - expected := "accepts 2 arg(s), received 3" - if got != expected { - t.Fatalf("Expected %q, got %q", expected, got) - } +func TestDEPRECATED_ExactValidArgs_WithInvalidCount(t *testing.T) { + _, e := executeCommand(newCommand(ExactValidArgs(2), true), "two", "three", "one") + expectError(e, t, "exact") } -func TestExactValidArgsWithInvalidArgs(t *testing.T) { - c := &Command{ - Use: "c", - Args: ExactValidArgs(1), - ValidArgs: []string{"one", "two"}, - Run: emptyRun, - } - - _, err := executeCommand(c, "three") - if err == nil { - t.Fatal("Expected an error") - } - - got := err.Error() - expected := `invalid argument "three" for "c"` - if got != expected { - t.Errorf("Expected: %q, got: %q", expected, got) - } +func TestDEPRECATED_ExactValidArgs_WithInvalidArgs(t *testing.T) { + _, e := executeCommand(newCommand(ExactValidArgs(2), true), "two", "a") + expectError(e, t, "valid") } // This test make sure we keep backwards-compatibility with respect From 64d74018525d9f19f450d09cdba29c52405de1b5 Mon Sep 17 00:00:00 2001 From: umarcor Date: Thu, 21 Mar 2019 21:56:13 +0100 Subject: [PATCH 5/7] args_test: use sub-tests for grouping based on the validator --- args_test.go | 310 +++++++++++++++++---------------------------------- 1 file changed, 101 insertions(+), 209 deletions(-) diff --git a/args_test.go b/args_test.go index 4c1fd42a..dfecb1a0 100644 --- a/args_test.go +++ b/args_test.go @@ -6,204 +6,126 @@ import ( "testing" ) -func newCommand(args PositionalArgs, withValid bool) *Command { +type argsTestcase struct { + exerr string // Expected error key (see map[string][string]) + args PositionalArgs // Args validator + wValid bool // Define `ValidArgs` in the command + rargs []string // Runtime args +} + +var errStrings = map[string]string{ + "invalid": `invalid argument "a" for "c"`, + "unknown": `unknown command "one" for "c"`, + "less": "requires at least 2 arg(s), only received 1", + "more": "accepts at most 2 arg(s), received 3", + "notexact": "accepts 2 arg(s), received 3", + "notinrange": "accepts between 2 and 4 arg(s), received 1", +} + +func (tc *argsTestcase) test(t *testing.T) { c := &Command{ Use: "c", - Args: args, + Args: tc.args, Run: emptyRun, } - if withValid { + if tc.wValid { c.ValidArgs = []string{"one", "two", "three"} } - return c -} -func expectSuccess(output string, err error, t *testing.T) { - if output != "" { - t.Errorf("Unexpected output: %v", output) - } - if err != nil { - t.Fatalf("Unexpected error: %v", err) + o, e := executeCommand(c, tc.rargs...) + + if len(tc.exerr) > 0 { + // Expect error + if e == nil { + t.Fatal("Expected an error") + } + expected, ok := errStrings[tc.exerr] + if !ok { + t.Errorf(`key "%s" is not found in map "errStrings"`, tc.exerr) + return + } + if got := e.Error(); got != expected { + t.Errorf("Expected: %q, got: %q", expected, got) + } + } else { + // Expect success + if o != "" { + t.Errorf("Unexpected output: %v", o) + } + if e != nil { + t.Fatalf("Unexpected error: %v", e) + } } } -func expectError(err error, t *testing.T, ex string) { - if err == nil { - t.Fatal("Expected an error") - } - expected := map[string]string{ - "valid": `invalid argument "a" for "c"`, - "no": `unknown command "one" for "c"`, - "min": "requires at least 2 arg(s), only received 1", - "max": "accepts at most 2 arg(s), received 3", - "exact": "accepts 2 arg(s), received 3", - "range": "accepts between 2 and 4 arg(s), received 1", - }[ex] - if got := err.Error(); got != expected { - t.Errorf("Expected: %q, got: %q", expected, got) +func testArgs(t *testing.T, tests map[string]argsTestcase) { + for name, tc := range tests { + t.Run(name, tc.test) } } -// NoArgs - -func TestNoArgs(t *testing.T) { - o, e := executeCommand(newCommand(NoArgs, false)) - expectSuccess(o, e, t) +func TestArgs_No(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | ": {"", NoArgs, false, []string{}}, + " | Arb": {"unknown", NoArgs, false, []string{"one"}}, + "Valid | Valid": {"unknown", NoArgs, true, []string{"one"}}, + }) } - -func TestNoArgs_WithArgs(t *testing.T) { - _, e := executeCommand(newCommand(NoArgs, false), "one") - expectError(e, t, "no") +func TestArgs_Arbitrary(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", ArbitraryArgs, false, []string{"a", "b"}}, + "Valid | Valid": {"", ArbitraryArgs, true, []string{"one", "two"}}, + "Valid | Invalid": {"invalid", ArbitraryArgs, true, []string{"a"}}, + }) } - -func TestNoArgs_WithArgs_WithValid(t *testing.T) { - _, e := executeCommand(newCommand(NoArgs, true), "one") - expectError(e, t, "no") +func TestArgs_MinimumN(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", MinimumNArgs(2), false, []string{"a", "b", "c"}}, + "Valid | Valid": {"", MinimumNArgs(2), true, []string{"one", "three"}}, + "Valid | Invalid": {"invalid", MinimumNArgs(2), true, []string{"a", "b"}}, + " | Less": {"less", MinimumNArgs(2), false, []string{"a"}}, + "Valid | Less": {"less", MinimumNArgs(2), true, []string{"one"}}, + "Valid | LessInvalid": {"invalid", MinimumNArgs(2), true, []string{"a"}}, + }) } - -// ArbitraryArgs - -func TestArbitraryArgs(t *testing.T) { - o, e := executeCommand(newCommand(ArbitraryArgs, false), "a", "b") - expectSuccess(o, e, t) +func TestArgs_MaximumN(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", MaximumNArgs(3), false, []string{"a", "b"}}, + "Valid | Valid": {"", MaximumNArgs(2), true, []string{"one", "three"}}, + "Valid | Invalid": {"invalid", MaximumNArgs(2), true, []string{"a", "b"}}, + " | More": {"more", MaximumNArgs(2), false, []string{"a", "b", "c"}}, + "Valid | More": {"more", MaximumNArgs(2), true, []string{"one", "three", "two"}}, + "Valid | MoreInvalid": {"invalid", MaximumNArgs(2), true, []string{"a", "b", "c"}}, + }) } - -func TestArbitraryArgs_WithValid(t *testing.T) { - o, e := executeCommand(newCommand(ArbitraryArgs, true), "one", "two") - expectSuccess(o, e, t) +func TestArgs_Exact(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", ExactArgs(3), false, []string{"a", "b", "c"}}, + "Valid | Valid": {"", ExactArgs(3), true, []string{"three", "one", "two"}}, + "Valid | Invalid": {"invalid", ExactArgs(3), true, []string{"three", "a", "two"}}, + " | InvalidCount": {"notexact", ExactArgs(2), false, []string{"a", "b", "c"}}, + "Valid | InvalidCount": {"notexact", ExactArgs(2), true, []string{"three", "one", "two"}}, + "Valid | InvalidCountInvalid": {"invalid", ExactArgs(2), true, []string{"three", "a", "two"}}, + }) } - -func TestArbitraryArgs_WithValid_WithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(ArbitraryArgs, true), "a") - expectError(e, t, "valid") +func TestArgs_Range(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", RangeArgs(2, 4), false, []string{"a", "b", "c"}}, + "Valid | Valid": {"", RangeArgs(2, 4), true, []string{"three", "one", "two"}}, + "Valid | Invalid": {"invalid", RangeArgs(2, 4), true, []string{"three", "a", "two"}}, + " | InvalidCount": {"notinrange", RangeArgs(2, 4), false, []string{"a"}}, + "Valid | InvalidCount": {"notinrange", RangeArgs(2, 4), true, []string{"two"}}, + "Valid | InvalidCountInvalid": {"invalid", RangeArgs(2, 4), true, []string{"a"}}, + }) } - -// MinimumNArgs - -func TestMinimumNArgs(t *testing.T) { - o, e := executeCommand(newCommand(MinimumNArgs(2), false), "a", "b", "c") - expectSuccess(o, e, t) -} - -func TestMinimumNArgs_WithValid(t *testing.T) { - o, e := executeCommand(newCommand(MinimumNArgs(2), true), "one", "three") - expectSuccess(o, e, t) -} - -func TestMinimumNArgs_WithValid_WithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(MinimumNArgs(2), true), "a", "b") - expectError(e, t, "valid") -} - -func TestMinimumNArgs_WithLessArgs(t *testing.T) { - _, e := executeCommand(newCommand(MinimumNArgs(2), false), "a") - expectError(e, t, "min") -} - -func TestMinimumNArgs_WithValid_WithLessArgs(t *testing.T) { - _, e := executeCommand(newCommand(MinimumNArgs(2), true), "one") - expectError(e, t, "min") -} - -func TestMinimumNArgs_WithValid_WithLessArgsWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(MinimumNArgs(2), true), "a") - expectError(e, t, "valid") -} - -// MaximumNArgs - -func TestMaximumNArgs(t *testing.T) { - o, e := executeCommand(newCommand(MaximumNArgs(3), false), "a", "b") - expectSuccess(o, e, t) -} - -func TestMaximumNArgs_WithValid(t *testing.T) { - o, e := executeCommand(newCommand(MaximumNArgs(2), true), "one", "three") - expectSuccess(o, e, t) -} - -func TestMaximumNArgs_WithValid_WithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(MaximumNArgs(2), true), "a", "b") - expectError(e, t, "valid") -} - -func TestMaximumNArgs_WithMoreArgs(t *testing.T) { - _, e := executeCommand(newCommand(MaximumNArgs(2), false), "a", "b", "c") - expectError(e, t, "max") -} - -func TestMaximumNArgs_WithValid_WithMoreArgs(t *testing.T) { - _, e := executeCommand(newCommand(MaximumNArgs(2), true), "one", "three", "two") - expectError(e, t, "max") -} - -func TestMaximumNArgs_WithValid_WithMoreArgsWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(MaximumNArgs(2), true), "a", "b", "c") - expectError(e, t, "valid") -} - -// ExactArgs - -func TestExactArgs(t *testing.T) { - o, e := executeCommand(newCommand(ExactArgs(3), false), "a", "b", "c") - expectSuccess(o, e, t) -} - -func TestExactArgs_WithValid(t *testing.T) { - o, e := executeCommand(newCommand(ExactArgs(3), true), "three", "one", "two") - expectSuccess(o, e, t) -} - -func TestExactArgs_WithValid_WithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(ExactArgs(3), true), "three", "a", "two") - expectError(e, t, "valid") -} - -func TestExactArgs_WithInvalidCount(t *testing.T) { - _, e := executeCommand(newCommand(ExactArgs(2), false), "a", "b", "c") - expectError(e, t, "exact") -} - -func TestExactArgs_WithValid_WithInvalidCount(t *testing.T) { - _, e := executeCommand(newCommand(ExactArgs(2), true), "three", "one", "two") - expectError(e, t, "exact") -} - -func TestExactArgs_WithValid_WithInvalidCountWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(ExactArgs(2), true), "three", "a", "two") - expectError(e, t, "valid") -} - -// RangeArgs - -func TestRangeArgs(t *testing.T) { - o, e := executeCommand(newCommand(RangeArgs(2, 4), false), "a", "b", "c") - expectSuccess(o, e, t) -} - -func TestRangeArgs_WithValid(t *testing.T) { - o, e := executeCommand(newCommand(RangeArgs(2, 4), true), "three", "one", "two") - expectSuccess(o, e, t) -} - -func TestRangeArgs_WithValid_WithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "three", "a", "two") - expectError(e, t, "valid") -} - -func TestRangeArgs_WithInvalidCount(t *testing.T) { - _, e := executeCommand(newCommand(RangeArgs(2, 4), false), "a") - expectError(e, t, "range") -} - -func TestRangeArgs_WithValid_WithInvalidCount(t *testing.T) { - _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "two") - expectError(e, t, "range") -} - -func TestRangeArgs_WithValid_WithInvalidCountWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "a") - expectError(e, t, "valid") +func TestArgs_DEPRECATED(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "OnlyValid | Valid | Valid": {"", OnlyValidArgs, true, []string{"one", "two"}}, + "OnlyValid | Valid | Invalid": {"invalid", OnlyValidArgs, true, []string{"a"}}, + "ExactValid | Valid | Valid": {"", ExactValidArgs(3), true, []string{"two", "three", "one"}}, + "ExactValid | Valid | InvalidCount": {"notexact", ExactValidArgs(2), true, []string{"two", "three", "one"}}, + "ExactValid | Valid | Invalid": {"invalid", ExactValidArgs(2), true, []string{"two", "a"}}, + }) } // Takes(No)Args @@ -312,36 +234,6 @@ func TestMatchAll(t *testing.T) { } } -// DEPRECATED - -func TestDEPRECATED_OnlyValidArgs(t *testing.T) { - o, e := executeCommand(newCommand(OnlyValidArgs, true), "one", "two") - expectSuccess(o, e, t) -} - -func TestDEPRECATED_OnlyValidArgs_WithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(OnlyValidArgs, true), "a") - expectError(e, t, "valid") -} - -func TestDEPRECATED_ExactValidArgs(t *testing.T) { - // Note that the order is not required to be the same: - // Definition: "one", "two", "three" - // Execution: "two", "three", "one" - o, e := executeCommand(newCommand(ExactValidArgs(3), true), "two", "three", "one") - expectSuccess(o, e, t) -} - -func TestDEPRECATED_ExactValidArgs_WithInvalidCount(t *testing.T) { - _, e := executeCommand(newCommand(ExactValidArgs(2), true), "two", "three", "one") - expectError(e, t, "exact") -} - -func TestDEPRECATED_ExactValidArgs_WithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(ExactValidArgs(2), true), "two", "a") - expectError(e, t, "valid") -} - // This test make sure we keep backwards-compatibility with respect // to the legacyArgs() function. // It makes sure the root command accepts arguments if it does not have From a9dbe74c87c3949b752a837040a421ec236d93e7 Mon Sep 17 00:00:00 2001 From: umarcor Date: Mon, 1 Feb 2021 11:49:41 +0100 Subject: [PATCH 6/7] args_test: add TestArgs_Nil --- args_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/args_test.go b/args_test.go index dfecb1a0..0c10a97a 100644 --- a/args_test.go +++ b/args_test.go @@ -71,6 +71,13 @@ func TestArgs_No(t *testing.T) { "Valid | Valid": {"unknown", NoArgs, true, []string{"one"}}, }) } +func TestArgs_Nil(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", nil, false, []string{"a", "b"}}, + "Valid | Valid": {"", nil, true, []string{"one", "two"}}, + "Valid | Invalid": {"invalid", nil, true, []string{"a"}}, + }) +} func TestArgs_Arbitrary(t *testing.T) { testArgs(t, map[string]argsTestcase{ " | Arb": {"", ArbitraryArgs, false, []string{"a", "b"}}, From 8825c7b3f94f34479ef540b6b5a011519b717141 Mon Sep 17 00:00:00 2001 From: umarcor Date: Tue, 19 Mar 2019 19:41:09 +0100 Subject: [PATCH 7/7] feat: add '[args]' and 'Valid Args:' to --help --- args.go | 22 +-- args_test.go | 425 ++++++++++++++++++++++++++++++++++++++++++------ command.go | 134 ++++++++++----- command_test.go | 42 ++--- 4 files changed, 498 insertions(+), 125 deletions(-) diff --git a/args.go b/args.go index aad963bd..359c92b7 100644 --- a/args.go +++ b/args.go @@ -26,27 +26,13 @@ func validateArgs(cmd *Command, args []string) error { return nil } -// Legacy arg validation has the following behaviour: -// - root commands with no subcommands can take arbitrary arguments -// - root commands with subcommands will do subcommand validity checking -// - subcommands will always accept arbitrary arguments -func legacyArgs(cmd *Command, args []string) error { - // no subcommand, always take args - if !cmd.HasSubCommands() { - return nil - } - - // root command with subcommands, do subcommand checking. - if !cmd.HasParent() && len(args) > 0 { - return fmt.Errorf("unknown command %q for %q%s", args[0], cmd.CommandPath(), cmd.findSuggestions(args[0])) - } - return nil -} - // NoArgs returns an error if any args are included. func NoArgs(cmd *Command, args []string) error { if len(args) > 0 { - return fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath()) + if cmd.HasAvailableSubCommands() { + return fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath()) + } + return fmt.Errorf("\"%s\" rejected; %q does not accept args", args[0], cmd.CommandPath()) } return nil } diff --git a/args_test.go b/args_test.go index 0c10a97a..d34dc1d5 100644 --- a/args_test.go +++ b/args_test.go @@ -1,19 +1,60 @@ package cobra import ( + "bytes" "fmt" "strings" "testing" ) +func executeUsage(c *Command) (string, error) { + buf := new(bytes.Buffer) + c.SetOutput(buf) + err := c.Usage() + return buf.String(), err +} + +func checkOutput(o string, t *testing.T, i string) { + str := map[rune]string{ + 'u': "Usage:", + 'h': "Run 'c --help' for usage", + 'c': "c [command]", + 'v': "Valid Args:", + 'a': "c [flags] [args]", + 'f': "c [flags]", + } + for _, x := range "uhcva" { + b := strings.Contains(i, string(x)) + if s := str[x]; b != strings.Contains(o, s) { + m := "Did not expect" + if b { + m = "Expected" + } + t.Errorf("%s to find '%s' in the output", m, s) + continue + } + if (x == 'a') && b { + return + } + } +} + +func expectErrorAndCheckOutput(t *testing.T, err error, err_k, o, i string) { + // expectError(err, t, err_k) + // checkOutput(o, t, i) +} + type argsTestcase struct { - exerr string // Expected error key (see map[string][string]) - args PositionalArgs // Args validator - wValid bool // Define `ValidArgs` in the command - rargs []string // Runtime args + exerr string // Expected error key (see map[string][string]) + args PositionalArgs // Args validator + wValid, wRun bool // Define `ValidArgs` in the command + rargs []string // Runtime args } var errStrings = map[string]string{ + "run": `command "c" is not runnable`, + "runsub": `command "c" is not runnable; please provide a subcmd`, + "no": `"one" rejected; "c" does not accept args`, "invalid": `invalid argument "a" for "c"`, "unknown": `unknown command "one" for "c"`, "less": "requires at least 2 arg(s), only received 1", @@ -22,17 +63,29 @@ var errStrings = map[string]string{ "notinrange": "accepts between 2 and 4 arg(s), received 1", } -func (tc *argsTestcase) test(t *testing.T) { +func newCmd(args PositionalArgs, wValid, wRun bool) *Command { c := &Command{ - Use: "c", - Args: tc.args, - Run: emptyRun, + Use: "c", + Short: "A generator", + Long: `Cobra is a CLI ...`, + //Run: emptyRun, } - if tc.wValid { + if args != nil { + c.Args = args + } + if wValid { c.ValidArgs = []string{"one", "two", "three"} } + if wRun { + c.Run = func(cmd *Command, args []string) { + //fmt.Println("RUN", args) + } + } + return c +} - o, e := executeCommand(c, tc.rargs...) +func (tc *argsTestcase) test(t *testing.T) { + o, e := executeCommand(newCmd(tc.args, tc.wValid, tc.wRun), tc.rargs...) if len(tc.exerr) > 0 { // Expect error @@ -66,72 +119,72 @@ func testArgs(t *testing.T, tests map[string]argsTestcase) { func TestArgs_No(t *testing.T) { testArgs(t, map[string]argsTestcase{ - " | ": {"", NoArgs, false, []string{}}, - " | Arb": {"unknown", NoArgs, false, []string{"one"}}, - "Valid | Valid": {"unknown", NoArgs, true, []string{"one"}}, + " | ": {"", NoArgs, false, true, []string{}}, + " | Arb": {"no", NoArgs, false, true, []string{"one"}}, + "Valid | Valid": {"no", NoArgs, true, true, []string{"one"}}, }) } func TestArgs_Nil(t *testing.T) { testArgs(t, map[string]argsTestcase{ - " | Arb": {"", nil, false, []string{"a", "b"}}, - "Valid | Valid": {"", nil, true, []string{"one", "two"}}, - "Valid | Invalid": {"invalid", nil, true, []string{"a"}}, + " | Arb": {"", nil, false, true, []string{"a", "b"}}, + "Valid | Valid": {"", nil, true, true, []string{"one", "two"}}, + "Valid | Invalid": {"invalid", nil, true, true, []string{"a"}}, }) } func TestArgs_Arbitrary(t *testing.T) { testArgs(t, map[string]argsTestcase{ - " | Arb": {"", ArbitraryArgs, false, []string{"a", "b"}}, - "Valid | Valid": {"", ArbitraryArgs, true, []string{"one", "two"}}, - "Valid | Invalid": {"invalid", ArbitraryArgs, true, []string{"a"}}, + " | Arb": {"", ArbitraryArgs, false, true, []string{"a", "b"}}, + "Valid | Valid": {"", ArbitraryArgs, true, true, []string{"one", "two"}}, + "Valid | Invalid": {"invalid", ArbitraryArgs, true, true, []string{"a"}}, }) } func TestArgs_MinimumN(t *testing.T) { testArgs(t, map[string]argsTestcase{ - " | Arb": {"", MinimumNArgs(2), false, []string{"a", "b", "c"}}, - "Valid | Valid": {"", MinimumNArgs(2), true, []string{"one", "three"}}, - "Valid | Invalid": {"invalid", MinimumNArgs(2), true, []string{"a", "b"}}, - " | Less": {"less", MinimumNArgs(2), false, []string{"a"}}, - "Valid | Less": {"less", MinimumNArgs(2), true, []string{"one"}}, - "Valid | LessInvalid": {"invalid", MinimumNArgs(2), true, []string{"a"}}, + " | Arb": {"", MinimumNArgs(2), false, true, []string{"a", "b", "c"}}, + "Valid | Valid": {"", MinimumNArgs(2), true, true, []string{"one", "three"}}, + "Valid | Invalid": {"invalid", MinimumNArgs(2), true, true, []string{"a", "b"}}, + " | Less": {"less", MinimumNArgs(2), false, true, []string{"a"}}, + "Valid | Less": {"less", MinimumNArgs(2), true, true, []string{"one"}}, + "Valid | LessInvalid": {"invalid", MinimumNArgs(2), true, true, []string{"a"}}, }) } func TestArgs_MaximumN(t *testing.T) { testArgs(t, map[string]argsTestcase{ - " | Arb": {"", MaximumNArgs(3), false, []string{"a", "b"}}, - "Valid | Valid": {"", MaximumNArgs(2), true, []string{"one", "three"}}, - "Valid | Invalid": {"invalid", MaximumNArgs(2), true, []string{"a", "b"}}, - " | More": {"more", MaximumNArgs(2), false, []string{"a", "b", "c"}}, - "Valid | More": {"more", MaximumNArgs(2), true, []string{"one", "three", "two"}}, - "Valid | MoreInvalid": {"invalid", MaximumNArgs(2), true, []string{"a", "b", "c"}}, + " | Arb": {"", MaximumNArgs(3), false, true, []string{"a", "b"}}, + "Valid | Valid": {"", MaximumNArgs(2), true, true, []string{"one", "three"}}, + "Valid | Invalid": {"invalid", MaximumNArgs(2), true, true, []string{"a", "b"}}, + " | More": {"more", MaximumNArgs(2), false, true, []string{"a", "b", "c"}}, + "Valid | More": {"more", MaximumNArgs(2), true, true, []string{"one", "three", "two"}}, + "Valid | MoreInvalid": {"invalid", MaximumNArgs(2), true, true, []string{"a", "b", "c"}}, }) } func TestArgs_Exact(t *testing.T) { testArgs(t, map[string]argsTestcase{ - " | Arb": {"", ExactArgs(3), false, []string{"a", "b", "c"}}, - "Valid | Valid": {"", ExactArgs(3), true, []string{"three", "one", "two"}}, - "Valid | Invalid": {"invalid", ExactArgs(3), true, []string{"three", "a", "two"}}, - " | InvalidCount": {"notexact", ExactArgs(2), false, []string{"a", "b", "c"}}, - "Valid | InvalidCount": {"notexact", ExactArgs(2), true, []string{"three", "one", "two"}}, - "Valid | InvalidCountInvalid": {"invalid", ExactArgs(2), true, []string{"three", "a", "two"}}, + " | Arb": {"", ExactArgs(3), false, true, []string{"a", "b", "c"}}, + "Valid | Valid": {"", ExactArgs(3), true, true, []string{"three", "one", "two"}}, + "Valid | Invalid": {"invalid", ExactArgs(3), true, true, []string{"three", "a", "two"}}, + " | InvalidCount": {"notexact", ExactArgs(2), false, true, []string{"a", "b", "c"}}, + "Valid | InvalidCount": {"notexact", ExactArgs(2), true, true, []string{"three", "one", "two"}}, + "Valid | InvalidCountInvalid": {"invalid", ExactArgs(2), true, true, []string{"three", "a", "two"}}, }) } func TestArgs_Range(t *testing.T) { testArgs(t, map[string]argsTestcase{ - " | Arb": {"", RangeArgs(2, 4), false, []string{"a", "b", "c"}}, - "Valid | Valid": {"", RangeArgs(2, 4), true, []string{"three", "one", "two"}}, - "Valid | Invalid": {"invalid", RangeArgs(2, 4), true, []string{"three", "a", "two"}}, - " | InvalidCount": {"notinrange", RangeArgs(2, 4), false, []string{"a"}}, - "Valid | InvalidCount": {"notinrange", RangeArgs(2, 4), true, []string{"two"}}, - "Valid | InvalidCountInvalid": {"invalid", RangeArgs(2, 4), true, []string{"a"}}, + " | Arb": {"", RangeArgs(2, 4), false, true, []string{"a", "b", "c"}}, + "Valid | Valid": {"", RangeArgs(2, 4), true, true, []string{"three", "one", "two"}}, + "Valid | Invalid": {"invalid", RangeArgs(2, 4), true, true, []string{"three", "a", "two"}}, + " | InvalidCount": {"notinrange", RangeArgs(2, 4), false, true, []string{"a"}}, + "Valid | InvalidCount": {"notinrange", RangeArgs(2, 4), true, true, []string{"two"}}, + "Valid | InvalidCountInvalid": {"invalid", RangeArgs(2, 4), true, true, []string{"a"}}, }) } func TestArgs_DEPRECATED(t *testing.T) { testArgs(t, map[string]argsTestcase{ - "OnlyValid | Valid | Valid": {"", OnlyValidArgs, true, []string{"one", "two"}}, - "OnlyValid | Valid | Invalid": {"invalid", OnlyValidArgs, true, []string{"a"}}, - "ExactValid | Valid | Valid": {"", ExactValidArgs(3), true, []string{"two", "three", "one"}}, - "ExactValid | Valid | InvalidCount": {"notexact", ExactValidArgs(2), true, []string{"two", "three", "one"}}, - "ExactValid | Valid | Invalid": {"invalid", ExactValidArgs(2), true, []string{"two", "a"}}, + "OnlyValid | Valid | Valid": {"", OnlyValidArgs, true, true, []string{"one", "two"}}, + "OnlyValid | Valid | Invalid": {"invalid", OnlyValidArgs, true, true, []string{"a"}}, + "ExactValid | Valid | Valid": {"", ExactValidArgs(3), true, true, []string{"two", "three", "one"}}, + "ExactValid | Valid | InvalidCount": {"notexact", ExactValidArgs(2), true, true, []string{"two", "three", "one"}}, + "ExactValid | Valid | Invalid": {"invalid", ExactValidArgs(2), true, true, []string{"two", "a"}}, }) } @@ -176,7 +229,7 @@ func TestChildTakesNoArgs(t *testing.T) { } got := err.Error() - expected := `unknown command "illegal" for "root child"` + expected := `"illegal" rejected; "root child" does not accept args` if !strings.Contains(got, expected) { t.Errorf("expected %q, got %q", expected, got) } @@ -193,6 +246,280 @@ func TestChildTakesArgs(t *testing.T) { } } +// NOTE 'c [command]' is not shown because this command does not have any subcommand +// NOTE 'Valid Args:' is not shown because this command is not runnable +// NOTE 'c [flags]' is not shown because this command is not runnable +func noRunChecks(t *testing.T, err error, err_k, o string) { + expectErrorAndCheckOutput(t, err, err_k, o, "u") +} + +// NoRun (no children) + +func TestArgs_NoRun(t *testing.T) { + tc := argsTestcase{"run", nil, false, false, []string{}} + t.Run("|", tc.test) + // noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_ArbValid(t *testing.T) { + tc := argsTestcase{"run", nil, false, false, []string{"one", "three"}} + t.Run("|", tc.test) + // noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_Invalid(t *testing.T) { + tc := argsTestcase{"run", nil, false, false, []string{"two", "a"}} + t.Run("|", tc.test) + //noRunChecks(t, e, "run", o) +} + +// NoRun (with children) +// NOTE 'Valid Args:' is not shown because this command is not runnable +// NOTE 'c [flags]' is not shown because this command is not runnable + +func TestArgs_NoRun_wChild(t *testing.T) { + c := newCmd(nil, false, false) + d := newCmd(nil, false, true) + c.AddCommand(d) + o, e := executeCommand(c) + expectErrorAndCheckOutput(t, e, "runsub", o, "uc") +} + +func TestArgs_NoRun_wChild_ArbValid(t *testing.T) { + c := newCmd(nil, false, false) + d := newCmd(nil, false, true) + c.AddCommand(d) + o, e := executeCommand(c, "one", "three") + expectErrorAndCheckOutput(t, e, "runsub", o, "h") +} + +func TestArgs_NoRun_wChild_Invalid(t *testing.T) { + c := newCmd(nil, false, false) + d := newCmd(nil, false, true) + c.AddCommand(d) + o, e := executeCommand(c, "one", "a") + expectErrorAndCheckOutput(t, e, "runsub", o, "h") +} + +// NoRun Args + +func TestArgs_NoRun_wArgs(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", ArbitraryArgs, false, false, []string{}}, + }) + //noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wArgs_ArbValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", ArbitraryArgs, false, false, []string{"one", "three"}}, + }) + //noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wArgs_Invalid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", ArbitraryArgs, false, false, []string{"two", "a"}}, + }) + //noRunChecks(t, e, "run", o) +} + +// NoRun ValidArgs + +func TestArgs_NoRun_wValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", nil, true, false, []string{}}, + }) + //noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wValid_ArbValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", nil, true, false, []string{"one", "three"}}, + }) + //noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wValid_Invalid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", nil, true, false, []string{"two", "a"}}, + }) + //noRunChecks(t, e, "run", o) +} + +// NoRun Args ValidArgs + +func TestArgs_NoRun_wArgswValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", ArbitraryArgs, true, false, []string{}}, + }) + // noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wArgswValid_ArbValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", ArbitraryArgs, true, false, []string{"one", "three"}}, + }) + // noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wArgswValid_Invalid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"run", ArbitraryArgs, true, false, []string{"two", "a"}}, + }) + // noRunChecks(t, e, "run", o) +} + +// Run (no children) +// NOTE 'c [command]' is not shown because this command does not have any subcommand +// NOTE 'Valid Args:' is not shown because ValidArgs is not defined + +func TestArgs_Run(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", nil, false, true, []string{}}, + }) + //o, e = executeUsage(c) + //checkOutput(o, t, "ua") +} + +func TestArgs_Run_ArbValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", nil, false, true, []string{"one", "three"}}, + }) + // o, e = executeUsage(c) + // checkOutput(o, t, "ua") +} + +func TestArgs_Run_Invalid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", nil, false, true, []string{"two", "a"}}, + }) + //o, e = executeUsage(c) + //checkOutput(o, t, "ua") +} + +// Run (with children) +// NOTE 'Valid Args:' is not shown because ValidArgs is not defined + +func TestArgs_Run_wChild(t *testing.T) { + c := newCmd(nil, false, true) + d := newCmd(nil, false, true) + c.AddCommand(d) + // o, e := executeCommand(c) + // expectSuccess(o, e, t) + o, _ := executeUsage(c) + checkOutput(o, t, "ucf") +} + +func TestArgs_Run_wChild_ArbValid(t *testing.T) { + c := newCmd(nil, false, true) + d := newCmd(nil, false, false) + c.AddCommand(d) + o, _ := executeCommand(c, "one", "three") + // expectError(e, t, "no") + // NOTE 'c [command]' is not shown because this command does not have any available subcommand + checkOutput(o, t, "uf") +} + +func TestArgs_Run_wChild_Invalid(t *testing.T) { + c := newCmd(nil, false, true) + d := newCmd(nil, false, false) + c.AddCommand(d) + o, _ := executeCommand(c, "one", "a") + // expectError(e, t, "no") + // NOTE 'c [command]' is not shown because this command does not have any available subcommand + checkOutput(o, t, "uf") +} + +// Run Args +// NOTE 'c [command]' is not shown because this command does not have any subcommand + +func TestArgs_Run_wArgs(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", ArbitraryArgs, false, true, []string{}}, + }) + // o, e = executeUsage(c) + // checkOutput(o, t, "ua") +} + +func TestArgs_Run_wArgs_ArbValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", ArbitraryArgs, false, true, []string{"one", "three"}}, + }) + // o, e = executeUsage(c) + // checkOutput(o, t, "ua") +} + +func TestArgs_Run_wArgs_Invalid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", ArbitraryArgs, false, true, []string{"two", "a"}}, + }) + // o, e = executeUsage(c) + // checkOutput(o, t, "ua") +} + +// Run ValidArgs +// NOTE 'c [command]' is not shown because this command does not have any subcommand + +func TestArgs_Run_wValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", nil, true, true, []string{}}, + }) + // o, e = executeUsage(c) + // checkOutput(o, t, "uva") +} + +func TestArgs_Run_wValid_ArbValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", nil, true, true, []string{"one", "three"}}, + }) + // o, e = executeUsage(c) + // checkOutput(o, t, "uva") +} + +func TestArgs_Run_wValid_Invalid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"invalid", nil, true, true, []string{"two", "a"}}, + }) + // checkOutput(o, t, "uva") +} + +// Run Args ValidArgs +// NOTE 'c [command]' is not shown because this command does not have any subcommand + +func TestArgs_Run_wArgswValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", ArbitraryArgs, true, true, []string{}}, + }) + //o, e = executeUsage(c) + //checkOutput(o, t, "uva") +} + +func TestArgs_Run_wArgswValid_ArbValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", ArbitraryArgs, true, true, []string{"one", "three"}}, + }) + //o, e = executeUsage(c) + //checkOutput(o, t, "uva") +} + +func TestArgs_Run_wArgswValid_Invalid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"invalid", ArbitraryArgs, true, true, []string{"two", "a"}}, + }) + //checkOutput(o, t, "uva") +} + +// + +func TestArgs_Run_wMinimumNArgs_ArbValid(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "|": {"", MinimumNArgs(2), false, true, []string{"one", "three"}}, + }) + //o, e = executeUsage(c) + //checkOutput(o, t, "ua") +} + func TestMatchAll(t *testing.T) { // Somewhat contrived example check that ensures there are exactly 3 // arguments, and each argument is exactly 2 bytes long. diff --git a/command.go b/command.go index c12c6c7c..1e165e79 100644 --- a/command.go +++ b/command.go @@ -29,6 +29,13 @@ import ( flag "github.com/spf13/pflag" ) +func ErrSubCommandRequired(s string) error { + return fmt.Errorf("command %s is not runnable; please provide a subcmd", s) +} +func ErrCommandNotRunnable(s string) error { + return fmt.Errorf(`command "%s" is not runnable`, s) +} + // FParseErrWhitelist configures Flag parse errors to be ignored type FParseErrWhitelist flag.ParseErrorsWhitelist @@ -505,7 +512,10 @@ func (c *Command) UsageTemplate() string { {{.CommandPath}} [command]{{end}}{{if gt (len .Aliases) 0}} Aliases: - {{.NameAndAliases}}{{end}}{{if .HasExample}} + {{.NameAndAliases}}{{end}}{{if (and .HasValidArgs .Runnable)}} + +Valid Args: + {{range .ValidArgs}}{{.}} {{end}}{{end}}{{if .HasExample}} Examples: {{.Example}}{{end}}{{if .HasAvailableSubCommands}} @@ -632,7 +642,7 @@ func isFlagArg(arg string) bool { // Find the target command given the args and command tree // Meant to be run on the highest node. Only searches down. -func (c *Command) Find(args []string) (*Command, []string, error) { +func (c *Command) Find(args []string) (*Command, []string) { var innerfind func(*Command, []string) (*Command, []string) innerfind = func(c *Command, innerArgs []string) (*Command, []string) { @@ -649,11 +659,13 @@ func (c *Command) Find(args []string) (*Command, []string, error) { return c, innerArgs } - commandFound, a := innerfind(c, args) - if commandFound.Args == nil { - return commandFound, a, legacyArgs(commandFound, stripFlags(a, commandFound)) + cF, a := innerfind(c, args) + // if Args is undefined and this is a root command with subcommands, + // do not accept arguments, unless ValidArgs is set + if cF.Args == nil && cF.HasSubCommands() && !cF.HasParent() && (len(cF.ValidArgs) == 0) { + cF.Args = NoArgs } - return commandFound, a, nil + return cF, a } func (c *Command) findSuggestions(arg string) string { @@ -694,7 +706,7 @@ func (c *Command) findNext(next string) *Command { // Traverse the command tree to find the command, and parse args for // each parent. -func (c *Command) Traverse(args []string) (*Command, []string, error) { +func (c *Command) Traverse(args []string) (*Command, []string) { flags := []string{} inFlag := false @@ -724,15 +736,15 @@ func (c *Command) Traverse(args []string) (*Command, []string, error) { cmd := c.findNext(arg) if cmd == nil { - return c, args, nil + return c, args } if err := c.ParseFlags(flags); err != nil { - return nil, args, err + return nil, append([]string{err.Error()}, args...) } return cmd.Traverse(args[i+1:]) } - return c, args, nil + return c, args } // SuggestionsFor provides suggestions for the typedName. @@ -828,7 +840,10 @@ func (c *Command) execute(a []string) (err error) { } if !c.Runnable() { - return flag.ErrHelp + if c.HasAvailableSubCommands() { + return ErrSubCommandRequired(c.Name()) + } + return ErrCommandNotRunnable(c.Name()) } c.preRun() @@ -949,7 +964,6 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { c.initDefaultCompletionCmd() args := c.args - // Workaround FAIL with "go test -v" or "cobra.test -test.v", see #155 if c.args == nil && filepath.Base(os.Args[0]) != "cobra.test" { args = os.Args[1:] @@ -959,35 +973,29 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { c.initCompleteCmd(args) var flags []string + f := c.Find if c.TraverseChildren { - cmd, flags, err = c.Traverse(args) - } else { - cmd, flags, err = c.Find(args) + f = c.Traverse } - if err != nil { - // If found parse to a subcommand and then failed, talk about the subcommand - if cmd != nil { - c = cmd + if cmd, flags = f(args); cmd != nil { + cmd.commandCalledAs.called = true + if cmd.commandCalledAs.name == "" { + cmd.commandCalledAs.name = cmd.Name() } if !c.SilenceErrors { c.PrintErrln("Error:", err.Error()) c.PrintErrf("Run '%v --help' for usage.\n", c.CommandPath()) } - return c, err + // We have to pass global context to children command + // if context is present on the parent command. + if cmd.ctx == nil { + cmd.ctx = c.ctx + } + err = cmd.execute(flags) + } else { + err = fmt.Errorf(flags[0]) } - cmd.commandCalledAs.called = true - if cmd.commandCalledAs.name == "" { - cmd.commandCalledAs.name = cmd.Name() - } - - // We have to pass global context to children command - // if context is present on the parent command. - if cmd.ctx == nil { - cmd.ctx = c.ctx - } - - err = cmd.execute(flags) if err != nil { // Always show help if requested, even if SilenceErrors is in // effect @@ -996,6 +1004,11 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, nil } + // Check if a shorter help hint should be shown instead of the full Usage() + if err := helpHint(cmd, flags, err.Error()); err != nil { + return cmd, err + } + // If root command has SilenceErrors flagged, // all subcommands should respect it if !cmd.SilenceErrors && !c.SilenceErrors { @@ -1011,6 +1024,25 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, err } +func helpHint(c *Command, fs []string, e string) error { + if len(fs) > 0 { + f := fs[0] + for _, s := range []string{"please provide a subcmd", "unknown command"} { + if strings.Contains(e, s) { + if s := c.findSuggestions(f); len(s) != 0 { + e += s + } + if !c.SilenceErrors { + c.Printf("Error: %s\n", e) + c.Printf("Run '%v --help' for usage.\n", c.CommandPath()) + } + return fmt.Errorf("%s", e) + } + } + } + return nil +} + // ValidateArgs returns an error if any positional args are not in the // `ValidArgs` field of `Command`. Then, run the `Args` validator, if // specified. @@ -1277,9 +1309,35 @@ func (c *Command) UseLine() string { if c.HasAvailableFlags() && !strings.Contains(useline, "[flags]") { useline += " [flags]" } + + useline += useLineArgs(c) + return useline } +// useLineArgs puts out '[args]' if a given command accepts positional args +func useLineArgs(c *Command) (s string) { + s = " [args]" + if c.Args == nil { + if !c.HasAvailableSubCommands() || c.HasParent() { + return + } + // if Args is undefined and this is a root command with subcommands, + // do not accept arguments, unless ValidArgs is set + if !c.HasParent() && c.HasAvailableSubCommands() && (len(c.ValidArgs) > 0) { + return + } + return "" + } + // Check if the Args validator is other than 'NoArgs' + err := c.Args(c, []string{"someUnexpectedIllegalArg"}) + nerr := NoArgs(c, []string{"someUnexpectedIllegalArg"}) + if err == nil || ((nerr != nil) && (err.Error() != nerr.Error())) { + return + } + return "" +} + // DebugFlags used to determine which flags have been assigned to which commands // and which persist. func (c *Command) DebugFlags() { @@ -1371,6 +1429,10 @@ func (c *Command) NameAndAliases() string { return strings.Join(append([]string{c.Name()}, c.Aliases...), ", ") } +func (c *Command) HasValidArgs() bool { + return len(c.ValidArgs) > 0 +} + // HasExample determines if the command has example. func (c *Command) HasExample() bool { return len(c.Example) > 0 @@ -1444,16 +1506,14 @@ func (c *Command) HasHelpSubCommands() bool { // HasAvailableSubCommands determines if a command has available sub commands that // need to be shown in the usage/help default template under 'available commands'. func (c *Command) HasAvailableSubCommands() bool { - // return true on the first found available (non deprecated/help/hidden) - // sub command + // return true on the first found available (non deprecated/help/hidden) subcmd for _, sub := range c.commands { if sub.IsAvailableCommand() { return true } } - - // the command either has no sub commands, or no available (non deprecated/help/hidden) - // sub commands + // the command either has no sub commands, + // or no available (non deprecated/help/hidden) subcmds return false } diff --git a/command_test.go b/command_test.go index 0446e3c1..43f92e4b 100644 --- a/command_test.go +++ b/command_test.go @@ -134,9 +134,7 @@ func TestRootExecuteUnknownCommand(t *testing.T) { rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun}) output, _ := executeCommand(rootCmd, "unknown") - expected := "Error: unknown command \"unknown\" for \"root\"\nRun 'root --help' for usage.\n" - if output != expected { t.Errorf("Expected:\n %q\nGot:\n %q\n", expected, output) } @@ -968,11 +966,13 @@ func TestHelpExecutedOnNonRunnableChild(t *testing.T) { rootCmd.AddCommand(childCmd) output, err := executeCommand(rootCmd, "child") - if err != nil { - t.Errorf("Unexpected error: %v", err) + + expected := `command "child" is not runnable` + if err.Error() != expected { + t.Errorf("Expected %q, got %q", expected, err.Error()) } - checkStringContains(t, output, childCmd.Long) + checkStringContains(t, output, "Usage:") } func TestVersionFlagExecuted(t *testing.T) { @@ -1820,9 +1820,9 @@ func TestTraverseWithParentFlags(t *testing.T) { rootCmd.AddCommand(childCmd) - c, args, err := rootCmd.Traverse([]string{"-b", "--str", "ok", "child", "--int"}) - if err != nil { - t.Errorf("Unexpected error: %v", err) + c, args := rootCmd.Traverse([]string{"-b", "--str", "ok", "child", "--int"}) + if c == nil { + t.Errorf("Unexpected error: %s", args[0]) } if len(args) != 1 && args[0] != "--add" { t.Errorf("Wrong args: %v", args) @@ -1840,9 +1840,9 @@ func TestTraverseNoParentFlags(t *testing.T) { childCmd.Flags().String("str", "", "") rootCmd.AddCommand(childCmd) - c, args, err := rootCmd.Traverse([]string{"child"}) - if err != nil { - t.Errorf("Unexpected error: %v", err) + c, args := rootCmd.Traverse([]string{"child"}) + if c == nil { + t.Errorf("Unexpected error: %v", args[0]) } if len(args) != 0 { t.Errorf("Wrong args %v", args) @@ -1861,13 +1861,13 @@ func TestTraverseWithBadParentFlags(t *testing.T) { expected := "unknown flag: --str" - c, _, err := rootCmd.Traverse([]string{"--str", "ok", "child"}) - if err == nil || !strings.Contains(err.Error(), expected) { - t.Errorf("Expected error, %q, got %q", expected, err) - } + c, args := rootCmd.Traverse([]string{"--str", "ok", "child"}) if c != nil { t.Errorf("Expected nil command") } + if !strings.Contains(args[0], expected) { + t.Errorf("Expected error, %q, got %q", expected, args[0]) + } } func TestTraverseWithBadChildFlag(t *testing.T) { @@ -1879,9 +1879,9 @@ func TestTraverseWithBadChildFlag(t *testing.T) { // Expect no error because the last commands args shouldn't be parsed in // Traverse. - c, args, err := rootCmd.Traverse([]string{"child", "--str"}) - if err != nil { - t.Errorf("Unexpected error: %v", err) + c, args := rootCmd.Traverse([]string{"child", "--str"}) + if c == nil { + t.Errorf("Unexpected error: %s", args[0]) } if len(args) != 1 && args[0] != "--str" { t.Errorf("Wrong args: %v", args) @@ -1902,9 +1902,9 @@ func TestTraverseWithTwoSubcommands(t *testing.T) { } subCmd.AddCommand(subsubCmd) - c, _, err := rootCmd.Traverse([]string{"sub", "subsub"}) - if err != nil { - t.Fatalf("Unexpected error: %v", err) + c, args := rootCmd.Traverse([]string{"sub", "subsub"}) + if c == nil { + t.Fatalf("Unexpected error: %v", args[0]) } if c.Name() != subsubCmd.Name() { t.Fatalf("Expected command: %q, got %q", subsubCmd.Name(), c.Name())