From bd96bdaf1b2956d171bf3b3d77893f9b43d7e3e5 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 1 Oct 2024 11:44:59 +0200 Subject: [PATCH] align "conflicting options" errors for consistency Signed-off-by: Sebastiaan van Stijn --- cli/command/cli.go | 4 ++-- cli/command/container/create_test.go | 29 ++++++++++++++++++++++++++++ cli/command/container/opts.go | 2 +- cli/command/container/opts_test.go | 6 ++---- cli/command/container/run.go | 2 +- cli/command/container/run_test.go | 29 ++++++++++++++++++++++++++++ cli/command/volume/create.go | 2 +- cli/command/volume/create_test.go | 2 +- 8 files changed, 66 insertions(+), 10 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 8d01abeb8c..dacd1a1109 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -272,7 +272,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...CLIOption) debug.Enable() } if opts.Context != "" && len(opts.Hosts) > 0 { - return errors.New("conflicting options: either specify --host or --context, not both") + return errors.New("conflicting options: cannot specify both --host and --context") } cli.options = opts @@ -299,7 +299,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...CLIOption) // NewAPIClientFromFlags creates a new APIClient from command line flags func NewAPIClientFromFlags(opts *cliflags.ClientOptions, configFile *configfile.ConfigFile) (client.APIClient, error) { if opts.Context != "" && len(opts.Hosts) > 0 { - return nil, errors.New("conflicting options: either specify --host or --context, not both") + return nil, errors.New("conflicting options: cannot specify both --host and --context") } storeConfig := DefaultContextStoreConfig() diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index 4485723296..0051a2bb78 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -195,6 +195,35 @@ func TestCreateContainerImagePullPolicyInvalid(t *testing.T) { } } +func TestCreateContainerValidateFlags(t *testing.T) { + for _, tc := range []struct { + name string + args []string + expectedErr string + }{ + { + name: "with invalid --attach value", + args: []string{"--attach", "STDINFO", "myimage"}, + expectedErr: `invalid argument "STDINFO" for "-a, --attach" flag: valid streams are STDIN, STDOUT and STDERR`, + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + cmd := NewCreateCommand(test.NewFakeCli(&fakeClient{})) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs(tc.args) + + err := cmd.Execute() + if tc.expectedErr != "" { + assert.Check(t, is.ErrorContains(err, tc.expectedErr)) + } else { + assert.Check(t, is.Nil(err)) + } + }) + } +} + func TestNewCreateCommandWithContentTrustErrors(t *testing.T) { testCases := []struct { name string diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index 2a49259d29..2106884898 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -704,7 +704,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con } if copts.autoRemove && !hostConfig.RestartPolicy.IsNone() { - return nil, errors.Errorf("Conflicting options: --restart and --rm") + return nil, errors.Errorf("conflicting options: cannot specify both --restart and --rm") } // only set this value if the user provided the flag, else it should default to nil diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index e03c26530e..c1bfedcca8 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -817,11 +817,9 @@ func TestParseRestartPolicy(t *testing.T) { } func TestParseRestartPolicyAutoRemove(t *testing.T) { - expected := "Conflicting options: --restart and --rm" _, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"}) //nolint:dogsled - if err == nil || err.Error() != expected { - t.Fatalf("Expected error %v, but got none", expected) - } + const expected = "conflicting options: cannot specify both --restart and --rm" + assert.Check(t, is.Error(err, expected)) } func TestParseHealth(t *testing.T) { diff --git a/cli/command/container/run.go b/cli/command/container/run.go index 12b06f7b1e..2767d1f488 100644 --- a/cli/command/container/run.go +++ b/cli/command/container/run.go @@ -131,7 +131,7 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption } } else { if copts.attach.Len() != 0 { - return errors.New("Conflicting options: -a and -d") + return errors.New("conflicting options: cannot specify both --attach and --detach") } config.AttachStdin = false diff --git a/cli/command/container/run_test.go b/cli/command/container/run_test.go index 390fc19d38..fad200ebae 100644 --- a/cli/command/container/run_test.go +++ b/cli/command/container/run_test.go @@ -23,6 +23,35 @@ import ( is "gotest.tools/v3/assert/cmp" ) +func TestRunValidateFlags(t *testing.T) { + for _, tc := range []struct { + name string + args []string + expectedErr string + }{ + { + name: "with conflicting --attach, --detach", + args: []string{"--attach", "stdin", "--detach", "myimage"}, + expectedErr: "conflicting options: cannot specify both --attach and --detach", + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + cmd := NewRunCommand(test.NewFakeCli(&fakeClient{})) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs(tc.args) + + err := cmd.Execute() + if tc.expectedErr != "" { + assert.Check(t, is.ErrorContains(err, tc.expectedErr)) + } else { + assert.Check(t, is.Nil(err)) + } + }) + } +} + func TestRunLabel(t *testing.T) { fakeCLI := test.NewFakeCli(&fakeClient{ createContainerFunc: func(_ *container.Config, _ *container.HostConfig, _ *network.NetworkingConfig, _ *specs.Platform, _ string) (container.CreateResponse, error) { diff --git a/cli/command/volume/create.go b/cli/command/volume/create.go index 15e1627cbb..0e4d035f9a 100644 --- a/cli/command/volume/create.go +++ b/cli/command/volume/create.go @@ -52,7 +52,7 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 1 { if options.name != "" { - return errors.Errorf("conflicting options: either specify --name or provide positional arg, not both") + return errors.Errorf("conflicting options: cannot specify a volume-name through both --name and as a positional arg") } options.name = args[0] } diff --git a/cli/command/volume/create_test.go b/cli/command/volume/create_test.go index a37f9655b1..3417fb689a 100644 --- a/cli/command/volume/create_test.go +++ b/cli/command/volume/create_test.go @@ -26,7 +26,7 @@ func TestVolumeCreateErrors(t *testing.T) { flags: map[string]string{ "name": "volumeName", }, - expectedError: "conflicting options: either specify --name or provide positional arg, not both", + expectedError: "conflicting options: cannot specify a volume-name through both --name and as a positional arg", }, { args: []string{"too", "many"},