Merge pull request #5488 from thaJeztah/align_errs

align "conflicting options" errors for consistency
docker · Oct 1, 2024 · f4fab2c · f4fab2c
2 parents 3907414 + bd96bda
commit f4fab2c
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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"},