From 51b7cf57e102be20a09835264c9d92155d602532 Mon Sep 17 00:00:00 2001 From: Albert Nigmatzianov Date: Sun, 14 May 2017 14:51:04 +0200 Subject: [PATCH] Fix tests so they give correct args (#445) * Fix tests so they give correct args Shell already deletes all quotes and unite args under quotes, so we don't need to test it. * Simplify stripFlags * Fix 'unused' and 'gosimple' complaints --- cobra_test.go | 179 ++++++++++++++++++++------------------------------ command.go | 39 +++++------ 2 files changed, 87 insertions(+), 131 deletions(-) diff --git a/cobra_test.go b/cobra_test.go index fcba209..576c97d 100644 --- a/cobra_test.go +++ b/cobra_test.go @@ -13,13 +13,12 @@ import ( "github.com/spf13/pflag" ) -var tp, te, tt, t1, tr []string +var tp, te, tt, tr []string var rootPersPre, echoPre, echoPersPre, timesPersPre []string var flagb1, flagb2, flagb3, flagbr, flagbp bool var flags1, flags2a, flags2b, flags3, outs string var flagi1, flagi2, flagi3, flagi4, flagir int -var globalFlag1 bool -var flagEcho, rootcalled bool +var rootcalled bool var versionUsed int const strtwoParentHelp = "help message for parent flag strtwo" @@ -227,36 +226,36 @@ type resulter struct { Command *Command } -func fullSetupTest(input string) resulter { +func fullSetupTest(args ...string) resulter { c := initializeWithRootCmd() - return fullTester(c, input) + return fullTester(c, args...) } -func noRRSetupTestSilenced(input string) resulter { +func noRRSetupTestSilenced(args ...string) resulter { c := initialize() c.SilenceErrors = true c.SilenceUsage = true - return fullTester(c, input) + return fullTester(c, args...) } -func noRRSetupTest(input string) resulter { +func noRRSetupTest(args ...string) resulter { c := initialize() - return fullTester(c, input) + return fullTester(c, args...) } -func rootOnlySetupTest(input string) resulter { +func rootOnlySetupTest(args ...string) resulter { c := initializeWithRootCmd() - return simpleTester(c, input) + return simpleTester(c, args...) } -func simpleTester(c *Command, input string) resulter { +func simpleTester(c *Command, args ...string) resulter { buf := new(bytes.Buffer) // Testing flag with invalid input c.SetOutput(buf) - c.SetArgs(strings.Split(input, " ")) + c.SetArgs(args) err := c.Execute() output := buf.String() @@ -264,11 +263,11 @@ func simpleTester(c *Command, input string) resulter { return resulter{err, output, c} } -func simpleTesterC(c *Command, input string) resulter { +func simpleTesterC(c *Command, args ...string) resulter { buf := new(bytes.Buffer) // Testing flag with invalid input c.SetOutput(buf) - c.SetArgs(strings.Split(input, " ")) + c.SetArgs(args) cmd, err := c.ExecuteC() output := buf.String() @@ -276,13 +275,13 @@ func simpleTesterC(c *Command, input string) resulter { return resulter{err, output, cmd} } -func fullTester(c *Command, input string) resulter { +func fullTester(c *Command, args ...string) resulter { buf := new(bytes.Buffer) // Testing flag with invalid input c.SetOutput(buf) cmdEcho.AddCommand(cmdTimes) c.AddCommand(cmdPrint, cmdEcho, cmdSubNoRun, cmdCustomFlags, cmdDeprecated) - c.SetArgs(strings.Split(input, " ")) + c.SetArgs(args) err := c.Execute() output := buf.String() @@ -332,7 +331,7 @@ func checkOutputContains(t *testing.T, c *Command, check string) { } func TestSingleCommand(t *testing.T) { - noRRSetupTest("print one two") + noRRSetupTest("print", "one", "two") if te != nil || tt != nil { t.Error("Wrong command called") @@ -346,7 +345,7 @@ func TestSingleCommand(t *testing.T) { } func TestChildCommand(t *testing.T) { - noRRSetupTest("echo times one two") + noRRSetupTest("echo", "times", "one", "two") if te != nil || tp != nil { t.Error("Wrong command called") @@ -360,7 +359,7 @@ func TestChildCommand(t *testing.T) { } func TestCommandAlias(t *testing.T) { - noRRSetupTest("say times one two") + noRRSetupTest("say", "times", "one", "two") if te != nil || tp != nil { t.Error("Wrong command called") @@ -375,7 +374,7 @@ func TestCommandAlias(t *testing.T) { func TestPrefixMatching(t *testing.T) { EnablePrefixMatching = true - noRRSetupTest("ech times one two") + noRRSetupTest("ech", "times", "one", "two") if te != nil || tp != nil { t.Error("Wrong command called") @@ -393,7 +392,7 @@ func TestPrefixMatching(t *testing.T) { func TestNoPrefixMatching(t *testing.T) { EnablePrefixMatching = false - noRRSetupTest("ech times one two") + noRRSetupTest("ech", "times", "one", "two") if !(tt == nil && te == nil && tp == nil) { t.Error("Wrong command called") @@ -402,7 +401,7 @@ func TestNoPrefixMatching(t *testing.T) { func TestAliasPrefixMatching(t *testing.T) { EnablePrefixMatching = true - noRRSetupTest("sa times one two") + noRRSetupTest("sa", "times", "one", "two") if te != nil || tp != nil { t.Error("Wrong command called") @@ -419,7 +418,7 @@ func TestAliasPrefixMatching(t *testing.T) { func TestChildSameName(t *testing.T) { c := initializeWithSameName() c.AddCommand(cmdPrint, cmdEcho) - c.SetArgs(strings.Split("print one two", " ")) + c.SetArgs([]string{"print", "one", "two"}) c.Execute() if te != nil || tt != nil { @@ -437,7 +436,7 @@ func TestGrandChildSameName(t *testing.T) { c := initializeWithSameName() cmdTimes.AddCommand(cmdPrint) c.AddCommand(cmdTimes) - c.SetArgs(strings.Split("times print one two", " ")) + c.SetArgs([]string{"times", "print", "one", "two"}) c.Execute() if te != nil || tt != nil { @@ -454,13 +453,13 @@ func TestGrandChildSameName(t *testing.T) { func TestUsage(t *testing.T) { x := fullSetupTest("help") checkResultContains(t, x, cmdRootWithRun.Use+" [flags]") - x = fullSetupTest("help customflags") + x = fullSetupTest("help", "customflags") checkResultContains(t, x, cmdCustomFlags.Use) checkResultOmits(t, x, cmdCustomFlags.Use+" [flags]") } func TestFlagLong(t *testing.T) { - noRRSetupTest("echo --intone=13 something -- here") + noRRSetupTest("echo", "--intone=13", "something", "--", "here") if cmdEcho.ArgsLenAtDash() != 1 { t.Errorf("expected argsLenAtDash: %d but got %d", 1, cmdRootNoRun.ArgsLenAtDash()) @@ -477,7 +476,7 @@ func TestFlagLong(t *testing.T) { } func TestFlagShort(t *testing.T) { - noRRSetupTest("echo -i13 -- something here") + noRRSetupTest("echo", "-i13", "--", "something", "here") if cmdEcho.ArgsLenAtDash() != 0 { t.Errorf("expected argsLenAtDash: %d but got %d", 0, cmdRootNoRun.ArgsLenAtDash()) @@ -492,7 +491,7 @@ func TestFlagShort(t *testing.T) { t.Errorf("default flag value changed, 234 expected, %d given", flagi2) } - noRRSetupTest("echo -i 13 something here") + noRRSetupTest("echo", "-i", "13", "something", "here") if strings.Join(te, " ") != "something here" { t.Errorf("flags didn't leave proper args remaining..%s given", te) @@ -504,7 +503,7 @@ func TestFlagShort(t *testing.T) { t.Errorf("default flag value changed, 234 expected, %d given", flagi2) } - noRRSetupTest("print -i99 one two") + noRRSetupTest("print", "-i99", "one", "two") if strings.Join(tp, " ") != "one two" { t.Errorf("flags didn't leave proper args remaining..%s given", tp) @@ -518,14 +517,14 @@ func TestFlagShort(t *testing.T) { } func TestChildCommandFlags(t *testing.T) { - noRRSetupTest("echo times -j 99 one two") + noRRSetupTest("echo", "times", "-j", "99", "one", "two") if strings.Join(tt, " ") != "one two" { t.Errorf("flags didn't leave proper args remaining..%s given", tt) } // Testing with flag that shouldn't be persistent - r := noRRSetupTest("echo times -j 99 -i77 one two") + r := noRRSetupTest("echo", "times", "-j", "99", "-i77", "one", "two") if r.Error == nil { t.Errorf("invalid flag should generate error") @@ -544,7 +543,7 @@ func TestChildCommandFlags(t *testing.T) { } // Testing with flag only existing on child - r = noRRSetupTest("echo -j 99 -i77 one two") + r = noRRSetupTest("echo", "-j", "99", "-i77", "one", "two") if r.Error == nil { t.Errorf("invalid flag should generate error") @@ -554,7 +553,7 @@ func TestChildCommandFlags(t *testing.T) { } // Testing with persistent flag overwritten by child - noRRSetupTest("echo times --strtwo=child one two") + noRRSetupTest("echo", "times", "--strtwo=child", "one", "two") if flags2b != "child" { t.Errorf("flag value should be child, %s given", flags2b) @@ -565,7 +564,7 @@ func TestChildCommandFlags(t *testing.T) { } // Testing flag with invalid input - r = noRRSetupTest("echo -i10E") + r = noRRSetupTest("echo", "-i10E") if r.Error == nil { t.Errorf("invalid input should generate error") @@ -576,7 +575,7 @@ func TestChildCommandFlags(t *testing.T) { } func TestTrailingCommandFlags(t *testing.T) { - x := fullSetupTest("echo two -x") + x := fullSetupTest("echo", "two", "-x") if x.Error == nil { t.Errorf("invalid flag should generate error") @@ -587,7 +586,7 @@ func TestInvalidSubcommandFlags(t *testing.T) { cmd := initializeWithRootCmd() cmd.AddCommand(cmdTimes) - result := simpleTester(cmd, "times --inttwo=2 --badflag=bar") + result := simpleTester(cmd, "times", "--inttwo=2", "--badflag=bar") // given that we are not checking here result.Error we check for // stock usage message checkResultContains(t, result, "cobra-test times [# times]") @@ -617,14 +616,14 @@ func TestSubcommandExecuteC(t *testing.T) { cmd.AddCommand(double, echo) - result := simpleTesterC(cmd, "double hello world") + result := simpleTesterC(cmd, "double", "hello", "world") checkResultContains(t, result, "hello world hello world") if result.Command.Name() != "double" { t.Errorf("invalid cmd returned from ExecuteC: should be 'double' but got %s", result.Command.Name()) } - result = simpleTesterC(cmd, "echo msg to be echoed") + result = simpleTesterC(cmd, "echo", "msg", "to", "be", "echoed") checkResultContains(t, result, "msg to be echoed") if result.Command.Name() != "echo" { @@ -650,16 +649,16 @@ func TestSubcommandArgEvaluation(t *testing.T) { } first.AddCommand(second) - result := simpleTester(cmd, "first second first third") + result := simpleTester(cmd, "first", "second", "first", "third") - expectedOutput := fmt.Sprintf("%v", []string{"first third"}) + expectedOutput := fmt.Sprint([]string{"first third"}) if result.Output != expectedOutput { t.Errorf("exptected %v, got %v", expectedOutput, result.Output) } } func TestPersistentFlags(t *testing.T) { - fullSetupTest("echo -s something -p more here") + fullSetupTest("echo", "-s", "something", "-p", "more", "here") // persistentFlag should act like normal flag on its own command if strings.Join(te, " ") != "more here" { @@ -673,7 +672,7 @@ func TestPersistentFlags(t *testing.T) { } // persistentFlag should act like normal flag on its own command - fullSetupTest("echo times -s again -c -p test here") + fullSetupTest("echo", "times", "-s", "again", "-c", "-p", "test", "here") if strings.Join(tt, " ") != "test here" { t.Errorf("flags didn't leave proper args remaining. %s given", tt) @@ -695,17 +694,17 @@ func TestHelpCommand(t *testing.T) { x := fullSetupTest("help") checkResultContains(t, x, cmdRootWithRun.Long) - x = fullSetupTest("help echo") + x = fullSetupTest("help", "echo") checkResultContains(t, x, cmdEcho.Long) - x = fullSetupTest("help echo times") + x = fullSetupTest("help", "echo", "times") checkResultContains(t, x, cmdTimes.Long) } func TestChildCommandHelp(t *testing.T) { - c := noRRSetupTest("print --help") + c := noRRSetupTest("print", "--help") checkResultContains(t, c, strtwoParentHelp) - r := noRRSetupTest("echo times --help") + r := noRRSetupTest("echo", "times", "--help") checkResultContains(t, r, strtwoChildHelp) } @@ -717,7 +716,7 @@ func TestNonRunChildHelp(t *testing.T) { func TestRunnableRootCommand(t *testing.T) { x := fullSetupTest("") - if rootcalled != true { + if !rootcalled { t.Errorf("Root Function was not called\n out:%v", x.Error) } } @@ -751,7 +750,6 @@ func TestVisitParents(t *testing.T) { } func TestRunnableRootCommandNilInput(t *testing.T) { - var emptyArg []string c := initializeWithRootCmd() buf := new(bytes.Buffer) @@ -759,23 +757,20 @@ func TestRunnableRootCommandNilInput(t *testing.T) { c.SetOutput(buf) cmdEcho.AddCommand(cmdTimes) c.AddCommand(cmdPrint, cmdEcho) - c.SetArgs(emptyArg) + c.SetArgs([]string{}) err := c.Execute() if err != nil { t.Errorf("Execute() failed with %v", err) } - if rootcalled != true { + if !rootcalled { t.Errorf("Root Function was not called") } } func TestRunnableRootCommandEmptyInput(t *testing.T) { - args := make([]string, 3) - args[0] = "" - args[1] = "--introot=12" - args[2] = "" + args := []string{"", "--introot=12", ""} c := initializeWithRootCmd() buf := new(bytes.Buffer) @@ -787,13 +782,13 @@ func TestRunnableRootCommandEmptyInput(t *testing.T) { c.Execute() - if rootcalled != true { - t.Errorf("Root Function was not called.\n\nOutput was:\n\n%s\n", buf) + if !rootcalled { + t.Errorf("Root Function was not called.\nOutput was:\n%s\n", buf) } } func TestInvalidSubcommandWhenArgsAllowed(t *testing.T) { - fullSetupTest("echo invalid-sub") + fullSetupTest("echo", "invalid-sub") if te[0] != "invalid-sub" { t.Errorf("Subcommand didn't work...") @@ -801,9 +796,9 @@ func TestInvalidSubcommandWhenArgsAllowed(t *testing.T) { } func TestRootFlags(t *testing.T) { - fullSetupTest("-i 17 -b") + fullSetupTest("-i", "17", "-b") - if flagbr != true { + if !flagbr { t.Errorf("flag value should be true, %v given", flagbr) } @@ -826,7 +821,7 @@ func TestRootHelp(t *testing.T) { t.Errorf("--help shouldn't display subcommand's usage, Got: \n %s", x.Output) } - x = fullSetupTest("echo --help") + x = fullSetupTest("echo", "--help") if strings.Contains(x.Output, cmdTimes.Use) { t.Errorf("--help shouldn't display subsubcommand's usage, Got: \n %s", x.Output) @@ -861,7 +856,6 @@ func TestFlagAccess(t *testing.T) { } func TestNoNRunnableRootCommandNilInput(t *testing.T) { - var args []string c := initialize() buf := new(bytes.Buffer) @@ -869,7 +863,7 @@ func TestNoNRunnableRootCommandNilInput(t *testing.T) { c.SetOutput(buf) cmdEcho.AddCommand(cmdTimes) c.AddCommand(cmdPrint, cmdEcho) - c.SetArgs(args) + c.SetArgs([]string{}) c.Execute() @@ -888,7 +882,7 @@ func TestRootNoCommandHelp(t *testing.T) { t.Errorf("--help shouldn't trigger an error, Got: \n %s", x.Output) } - x = rootOnlySetupTest("echo --help") + x = rootOnlySetupTest("echo", "--help") checkResultOmits(t, x, "Available Commands:") checkResultOmits(t, x, "for more information about a command") @@ -906,7 +900,7 @@ func TestRootUnknownCommand(t *testing.T) { t.Errorf("Unexpected response.\nExpecting to be:\n %q\nGot:\n %q\n", s, r.Output) } - r = noRRSetupTest("--strtwo=a bogus") + r = noRRSetupTest("--strtwo=a", "bogus") if r.Output != s { t.Errorf("Unexpected response.\nExpecting to be:\n %q\nGot:\n %q\n", s, r.Output) } @@ -919,7 +913,7 @@ func TestRootUnknownCommandSilenced(t *testing.T) { t.Errorf("Unexpected response.\nExpecting to be: \n\"\"\n Got:\n %q\n", r.Output) } - r = noRRSetupTestSilenced("--strtwo=a bogus") + r = noRRSetupTestSilenced("--strtwo=a", "bogus") if r.Output != "" { t.Errorf("Unexpected response.\nExpecting to be:\n\"\"\nGot:\n %q\n", r.Output) } @@ -966,56 +960,27 @@ func TestRootSuggestions(t *testing.T) { func TestFlagsBeforeCommand(t *testing.T) { // short without space - x := fullSetupTest("-i10 echo") + x := fullSetupTest("-i10", "echo") if x.Error != nil { t.Errorf("Valid Input shouldn't have errors, got:\n %q", x.Error) } - // short (int) with equals - // It appears that pflags doesn't support this... - // Commenting out until support can be added - - //x = noRRSetupTest("echo -i=10") - //if x.Error != nil { - //t.Errorf("Valid Input shouldn't have errors, got:\n %s", x.Error) - //} + x = noRRSetupTest("echo", "-i=10") + if x.Error != nil { + t.Errorf("Valid Input shouldn't have errors, got:\n %s", x.Error) + } // long with equals - x = noRRSetupTest("--intone=123 echo one two") + x = noRRSetupTest("--intone=123", "echo", "one", "two") if x.Error != nil { t.Errorf("Valid Input shouldn't have errors, got:\n %s", x.Error) } // With parsing error properly reported - x = fullSetupTest("-i10E echo") + x = fullSetupTest("-i10E", "echo") if !strings.Contains(x.Error.Error(), "invalid syntax") { t.Errorf("Wrong error message displayed, \n %s", x.Error) } - - //With quotes - x = fullSetupTest("-s=\"walking\" echo") - if x.Error != nil { - t.Errorf("Valid Input shouldn't have errors, got:\n %q", x.Error) - } - - //With quotes and space - x = fullSetupTest("-s=\"walking fast\" echo") - if x.Error != nil { - t.Errorf("Valid Input shouldn't have errors, got:\n %q", x.Error) - } - - //With inner quote - x = fullSetupTest("-s=\"walking \\\"Inner Quote\\\" fast\" echo") - if x.Error != nil { - t.Errorf("Valid Input shouldn't have errors, got:\n %q", x.Error) - } - - //With quotes and space - x = fullSetupTest("-s=\"walking \\\"Inner Quote\\\" fast\" echo") - if x.Error != nil { - t.Errorf("Valid Input shouldn't have errors, got:\n %q", x.Error) - } - } func TestRemoveCommand(t *testing.T) { @@ -1081,7 +1046,7 @@ func TestDeprecatedSub(t *testing.T) { } func TestPreRun(t *testing.T) { - noRRSetupTest("echo one two") + noRRSetupTest("echo", "one", "two") if echoPre == nil || echoPersPre == nil { t.Error("PreRun or PersistentPreRun not called") } @@ -1089,7 +1054,7 @@ func TestPreRun(t *testing.T) { t.Error("Wrong *Pre functions called!") } - noRRSetupTest("echo times one two") + noRRSetupTest("echo", "times", "one", "two") if timesPersPre == nil { t.Error("PreRun or PersistentPreRun not called") } @@ -1097,7 +1062,7 @@ func TestPreRun(t *testing.T) { t.Error("Wrong *Pre functions called!") } - noRRSetupTest("print one two") + noRRSetupTest("print", "one", "two") if rootPersPre == nil { t.Error("Parent PersistentPreRun not called but should not have been") } @@ -1115,10 +1080,10 @@ func TestPeristentPreRunPropagation(t *testing.T) { // Now add cmdPrint to rootCmd rootCmd.AddCommand(cmdPrint) - rootCmd.SetArgs(strings.Split("print echosub lala", " ")) + rootCmd.SetArgs([]string{"print", "echosub", "lala"}) rootCmd.Execute() - if rootPersPre == nil || len(rootPersPre) == 0 || rootPersPre[0] != "lala" { + if len(rootPersPre) == 0 || rootPersPre[0] != "lala" { t.Error("RootCmd PersistentPreRun not called but should have been") } } diff --git a/command.go b/command.go index 3be5554..01d9683 100644 --- a/command.go +++ b/command.go @@ -430,37 +430,28 @@ func stripFlags(args []string, c *Command) []string { c.mergePersistentFlags() commands := []string{} - inQuote := false flags := c.Flags() Loop: for len(args) > 0 { s := args[0] args = args[1:] - if !inQuote { - switch { - case strings.HasPrefix(s, "\"") || strings.Contains(s, "=\""): - inQuote = true - case strings.HasPrefix(s, "--") && !strings.Contains(s, "=") && !hasNoOptDefVal(s[2:], flags): - // If '--flag arg' then - // delete arg from args. - fallthrough // (do the same as below) - case strings.HasPrefix(s, "-") && !strings.Contains(s, "=") && len(s) == 2 && !shortHasNoOptDefVal(s[1:], flags): - // If '-f arg' then - // delete 'arg' from args or break the loop if len(args) <= 1. - if len(args) <= 1 { - break Loop - } else { - args = args[1:] - continue - } - case s != "" && !strings.HasPrefix(s, "-"): - commands = append(commands, s) + switch { + case strings.HasPrefix(s, "--") && !strings.Contains(s, "=") && !hasNoOptDefVal(s[2:], flags): + // If '--flag arg' then + // delete arg from args. + fallthrough // (do the same as below) + case strings.HasPrefix(s, "-") && !strings.Contains(s, "=") && len(s) == 2 && !shortHasNoOptDefVal(s[1:], flags): + // If '-f arg' then + // delete 'arg' from args or break the loop if len(args) <= 1. + if len(args) <= 1 { + break Loop + } else { + args = args[1:] + continue } - } - - if strings.HasSuffix(s, "\"") && !strings.HasSuffix(s, "\\\"") { - inQuote = false + case s != "" && !strings.HasPrefix(s, "-"): + commands = append(commands, s) } }