From d57175aa2e42c2e5d4cc14bf0d66d5779ccd1617 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 26 Apr 2019 10:37:26 +0100 Subject: [PATCH 1/4] Move subtests of TestGlobalHelp into actual subtests Signed-off-by: Ian Campbell --- e2e/cli-plugins/help_test.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/e2e/cli-plugins/help_test.go b/e2e/cli-plugins/help_test.go index 8e57ccc876..f023b0f2f1 100644 --- a/e2e/cli-plugins/help_test.go +++ b/e2e/cli-plugins/help_test.go @@ -74,18 +74,22 @@ func TestGlobalHelp(t *testing.T) { assert.Assert(t, is.Equal(badmetacount, 1)) // Running with `--help` should produce the same. - res2 := icmd.RunCmd(run("--help")) - res2.Assert(t, icmd.Expected{ - ExitCode: 0, + t.Run("help_flag", func(t *testing.T) { + res2 := icmd.RunCmd(run("--help")) + res2.Assert(t, icmd.Expected{ + ExitCode: 0, + }) + assert.Assert(t, is.Equal(res2.Stdout(), res.Stdout())) + assert.Assert(t, is.Equal(res2.Stderr(), "")) }) - assert.Assert(t, is.Equal(res2.Stdout(), res.Stdout())) - assert.Assert(t, is.Equal(res2.Stderr(), "")) // Running just `docker` (without `help` nor `--help`) should produce the same thing, except on Stderr. - res2 = icmd.RunCmd(run()) - res2.Assert(t, icmd.Expected{ - ExitCode: 0, + t.Run("bare", func(t *testing.T) { + res2 := icmd.RunCmd(run()) + res2.Assert(t, icmd.Expected{ + ExitCode: 0, + }) + assert.Assert(t, is.Equal(res2.Stdout(), "")) + assert.Assert(t, is.Equal(res2.Stderr(), res.Stdout())) }) - assert.Assert(t, is.Equal(res2.Stdout(), "")) - assert.Assert(t, is.Equal(res2.Stderr(), res.Stdout())) } From 79a75da0fd97b02311676028c3406b242c785f7c Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 26 Apr 2019 15:09:09 +0100 Subject: [PATCH 2/4] Hide experimental builtin commands in help output on unknown flag. Previously `docker --badopt` would always include experimental commands even if experimental was not enabled. Signed-off-by: Ian Campbell --- cmd/docker/docker.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 22b978de9b..f522a29a03 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -79,6 +79,9 @@ func setFlagErrorFunc(dockerCli *command.DockerCli, cmd *cobra.Command) { if err := isSupported(cmd, dockerCli); err != nil { return err } + if err := hideUnsupportedFeatures(cmd, dockerCli); err != nil { + return err + } return flagErrorFunc(cmd, err) }) } From 40a6cf7c477cf328134b0b8dcdbd9a09d02f918b Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 26 Apr 2019 15:08:22 +0100 Subject: [PATCH 3/4] Include CLI plugins in help output on unknown flag. Previously `docker --badopt` output would not include CLI plugins. Fixes #1813 Signed-off-by: Ian Campbell --- cmd/docker/docker.go | 3 +++ e2e/cli-plugins/help_test.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index f522a29a03..5b0fffb004 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -76,6 +76,9 @@ func setFlagErrorFunc(dockerCli *command.DockerCli, cmd *cobra.Command) { // is called. flagErrorFunc := cmd.FlagErrorFunc() cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + if err := pluginmanager.AddPluginCommandStubs(dockerCli, cmd.Root()); err != nil { + return err + } if err := isSupported(cmd, dockerCli); err != nil { return err } diff --git a/e2e/cli-plugins/help_test.go b/e2e/cli-plugins/help_test.go index f023b0f2f1..b73054d4a3 100644 --- a/e2e/cli-plugins/help_test.go +++ b/e2e/cli-plugins/help_test.go @@ -92,4 +92,18 @@ func TestGlobalHelp(t *testing.T) { assert.Assert(t, is.Equal(res2.Stdout(), "")) assert.Assert(t, is.Equal(res2.Stderr(), res.Stdout())) }) + + t.Run("badopt", func(t *testing.T) { + // Running `docker --badopt` should also produce the + // same thing, give or take the leading error message + // and a trailing carriage return (due to main() using + // Println in the error case). + res2 := icmd.RunCmd(run("--badopt")) + res2.Assert(t, icmd.Expected{ + ExitCode: 125, + }) + assert.Assert(t, is.Equal(res2.Stdout(), "")) + exp := "unknown flag: --badopt\nSee 'docker --help'.\n" + res.Stdout() + "\n" + assert.Assert(t, is.Equal(res2.Stderr(), exp)) + }) } From 7d0645c5fe817ca4e8677583e845b95f6723fe97 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 26 Apr 2019 15:43:03 +0100 Subject: [PATCH 4/4] Use command.Cli instead of command.DockerCli The linter is complaining: cmd/docker/docker.go:72:23:warning: dockerCli can be github.com/docker/cli/cli/command.Cli (interfacer) Unclear precisely which change in the preceeding commits caused it to notice this possibility. Signed-off-by: Ian Campbell --- cmd/docker/docker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 5b0fffb004..dafaea0703 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -69,7 +69,7 @@ func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand { return cli.NewTopLevelCommand(cmd, dockerCli, opts, flags) } -func setFlagErrorFunc(dockerCli *command.DockerCli, cmd *cobra.Command) { +func setFlagErrorFunc(dockerCli command.Cli, cmd *cobra.Command) { // When invoking `docker stack --nonsense`, we need to make sure FlagErrorFunc return appropriate // output if the feature is not supported. // As above cli.SetupRootCommand(cmd) have already setup the FlagErrorFunc, we will add a pre-check before the FlagErrorFunc