Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

args_test refactorization after generalizing ValidArgs #841

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

args_test refactorization after generalizing ValidArgs #841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2cd7871
feat: generalize ValidArgs; use it implicitly with any validator
umarcor Mar 18, 2019
78048ea
command/ValidateArgs: check ValidArgs regardless of Args being nil
umarcor Feb 1, 2021
d4d0679
add expectError helper function to args_test
umarcor Mar 18, 2019
d507a44
args_test: style
umarcor Mar 19, 2019
64d7401
args_test: use sub-tests for grouping based on the validator
umarcor Mar 21, 2019
a9dbe74
args_test: add TestArgs_Nil
umarcor Feb 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 34 additions & 31 deletions args.go
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
268 changes: 108 additions & 160 deletions args_test.go
View file
Open in desktop
Expand Up @@ -6,188 +6,136 @@ import (
"testing"
)

func getCommand(args PositionalArgs, withValid bool) *Command {
c := &Command{
Use: "c",
Args: args,
Run: emptyRun,
}
if withValid {
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)
}
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
}

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 {
t.Errorf("Expected: %q, got: %q", expected, got)
}
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 noArgsWithArgs(err error, t *testing.T) {
if err == nil {
t.Fatal("Expected an error")
}
got := err.Error()
expected := `unknown command "illegal" 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")
func (tc *argsTestcase) test(t *testing.T) {
c := &Command{
Use: "c",
Args: tc.args,
Run: emptyRun,
}
got := err.Error()
expected := "accepts at most 2 arg(s), received 3"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
if tc.wValid {
c.ValidArgs = []string{"one", "two", "three"}
}
}

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)
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 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)
func testArgs(t *testing.T, tests map[string]argsTestcase) {
for name, tc := range tests {
t.Run(name, tc.test)
}
}

func TestNoArgs(t *testing.T) {
c := getCommand(NoArgs, false)
output, err := executeCommand(c)
expectSuccess(output, err, t)
}

func TestNoArgsWithArgs(t *testing.T) {
c := getCommand(NoArgs, false)
_, err := executeCommand(c, "illegal")
noArgsWithArgs(err, t)
}

func TestOnlyValidArgs(t *testing.T) {
c := getCommand(OnlyValidArgs, true)
output, err := executeCommand(c, "one", "two")
expectSuccess(output, err, t)
}

func TestOnlyValidArgsWithInvalidArgs(t *testing.T) {
c := getCommand(OnlyValidArgs, true)
_, err := executeCommand(c, "a")
validWithInvalidArgs(err, 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 TestArbitraryArgs(t *testing.T) {
c := getCommand(ArbitraryArgs, false)
output, err := executeCommand(c, "a", "b")
expectSuccess(output, err, t)
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 TestMinimumNArgs(t *testing.T) {
c := getCommand(MinimumNArgs(2), false)
output, err := executeCommand(c, "a", "b", "c")
expectSuccess(output, err, t)
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 TestMinimumNArgsWithLessArgs(t *testing.T) {
c := getCommand(MinimumNArgs(2), false)
_, err := executeCommand(c, "a")
minimumNArgsWithLessArgs(err, t)
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"}},
})
}

func TestMaximumNArgs(t *testing.T) {
c := getCommand(MaximumNArgs(3), false)
output, err := executeCommand(c, "a", "b")
expectSuccess(output, err, 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 TestMaximumNArgsWithMoreArgs(t *testing.T) {
c := getCommand(MaximumNArgs(2), false)
_, err := executeCommand(c, "a", "b", "c")
maximumNArgsWithMoreArgs(err, 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 TestExactArgs(t *testing.T) {
c := getCommand(ExactArgs(3), false)
output, err := executeCommand(c, "a", "b", "c")
expectSuccess(output, err, t)
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"}},
})
}

func TestExactArgsWithInvalidCount(t *testing.T) {
c := getCommand(ExactArgs(2), false)
_, err := executeCommand(c, "a", "b", "c")
exactArgsWithInvalidCount(err, t)
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"}},
})
}

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)
_, err := executeCommand(c, "three", "one", "two")
exactArgsWithInvalidCount(err, t)
}

func TestExactValidArgsWithInvalidArgs(t *testing.T) {
c := getCommand(ExactValidArgs(3), true)
_, err := executeCommand(c, "three", "a", "two")
validWithInvalidArgs(err, t)
}

func TestRangeArgs(t *testing.T) {
c := getCommand(RangeArgs(2, 4), false)
output, err := executeCommand(c, "a", "b", "c")
expectSuccess(output, err, t)
}

func TestRangeArgsWithInvalidCount(t *testing.T) {
c := getCommand(RangeArgs(2, 4), false)
_, err := executeCommand(c, "a")
rangeArgsWithInvalidCount(err, t)
}
// Takes(No)Args

func TestRootTakesNoArgs(t *testing.T) {
rootCmd := &Command{Use: "root", Run: emptyRun}
Expand Down
2 changes: 1 addition & 1 deletion bash_completions_test.go
View file
Open in desktop
Expand Up @@ -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.",
Expand Down