cli/command/swarm: minor cleanups: use Println, rename vars

- use Println to print newline instead of custom format
- use apiClient instead of client for the API client to
  prevent shadowing imports.
- use dockerCLI with Go's standard camelCase casing.
- suppress some errors to make my IDE and linters happier
- fix some tests to work with "go test -update"
- rewrite TestSwarmInit to use sub-tests

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cli/command/swarm/ca_test.go

@@ -143,9 +143,10 @@ func TestDisplayTrustRootInvalidFlags(t *testing.T) {
 					}, nil
 				},
 			}))
-		assert.Check(t, cmd.Flags().Parse(testCase.args))
+		cmd.SetArgs([]string{})
 		cmd.SetOut(io.Discard)
 		cmd.SetErr(io.Discard)
+		assert.Check(t, cmd.Flags().Parse(testCase.args))
 		assert.ErrorContains(t, cmd.Execute(), testCase.errorMsg)
 	}
 }

cli/command/swarm/init.go

@@ -65,8 +65,8 @@ func newInitCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func runInit(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, opts initOptions) error {
+func runInit(ctx context.Context, dockerCLI command.Cli, flags *pflag.FlagSet, opts initOptions) error {
-	client := dockerCli.Client()
+	apiClient := dockerCLI.Client()
 
 	defaultAddrPool := make([]string, 0, len(opts.defaultAddrPools))
 	for _, p := range opts.defaultAddrPools {
@@ -93,7 +93,7 @@ func runInit(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, o
 		}
 	}
 
-	nodeID, err := client.SwarmInit(ctx, req)
+	nodeID, err := apiClient.SwarmInit(ctx, req)
 	if err != nil {
 		if strings.Contains(err.Error(), "could not choose an IP address to advertise") || strings.Contains(err.Error(), "could not find the system's IP address") {
 			return errors.New(err.Error() + " - specify one with --advertise-addr")
@@ -101,20 +101,20 @@ func runInit(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, o
 		return err
 	}
 
-	fmt.Fprintf(dockerCli.Out(), "Swarm initialized: current node (%s) is now a manager.\n\n", nodeID)
+	_, _ = fmt.Fprintf(dockerCLI.Out(), "Swarm initialized: current node (%s) is now a manager.\n\n", nodeID)
 
-	if err := printJoinCommand(ctx, dockerCli, nodeID, true, false); err != nil {
+	if err := printJoinCommand(ctx, dockerCLI, nodeID, true, false); err != nil {
 		return err
 	}
 
-	fmt.Fprint(dockerCli.Out(), "To add a manager to this swarm, run 'docker swarm join-token manager' and follow the instructions.\n\n")
+	_, _ = fmt.Fprintln(dockerCLI.Out(), "To add a manager to this swarm, run 'docker swarm join-token manager' and follow the instructions.")
 
 	if req.AutoLockManagers {
-		unlockKeyResp, err := client.SwarmGetUnlockKey(ctx)
+		unlockKeyResp, err := apiClient.SwarmGetUnlockKey(ctx)
 		if err != nil {
 			return errors.Wrap(err, "could not fetch unlock key")
 		}
-		printUnlockCommand(dockerCli.Out(), unlockKeyResp.UnlockKey)
+		printUnlockCommand(dockerCLI.Out(), unlockKeyResp.UnlockKey)
 	}
 
 	return nil

cli/command/swarm/init_test.go

@@ -71,11 +71,12 @@ func TestSwarmInitErrorOnAPIFailure(t *testing.T) {
 					swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc,
 					nodeInspectFunc:       tc.nodeInspectFunc,
 				}))
-			for key, value := range tc.flags {
+			cmd.SetArgs([]string{})
-				cmd.Flags().Set(key, value)
-			}
 			cmd.SetOut(io.Discard)
 			cmd.SetErr(io.Discard)
+			for k, v := range tc.flags {
+				assert.Check(t, cmd.Flags().Set(k, v))
+			}
 			assert.Error(t, cmd.Execute(), tc.expectedError)
 		})
 	}
@@ -112,17 +113,22 @@ func TestSwarmInit(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		cli := test.NewFakeCli(&fakeClient{
+		t.Run(tc.name, func(t *testing.T) {
-			swarmInitFunc:         tc.swarmInitFunc,
+			cli := test.NewFakeCli(&fakeClient{
-			swarmInspectFunc:      tc.swarmInspectFunc,
+				swarmInitFunc:         tc.swarmInitFunc,
-			swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc,
+				swarmInspectFunc:      tc.swarmInspectFunc,
-			nodeInspectFunc:       tc.nodeInspectFunc,
+				swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc,
+				nodeInspectFunc:       tc.nodeInspectFunc,
+			})
+			cmd := newInitCommand(cli)
+			cmd.SetArgs([]string{})
+			cmd.SetOut(io.Discard)
+			cmd.SetErr(io.Discard)
+			for k, v := range tc.flags {
+				assert.Check(t, cmd.Flags().Set(k, v))
+			}
+			assert.NilError(t, cmd.Execute())
+			golden.Assert(t, cli.OutBuffer().String(), fmt.Sprintf("init-%s.golden", tc.name))
 		})
-		cmd := newInitCommand(cli)
-		for key, value := range tc.flags {
-			cmd.Flags().Set(key, value)
-		}
-		assert.NilError(t, cmd.Execute())
-		golden.Assert(t, cli.OutBuffer().String(), fmt.Sprintf("init-%s.golden", tc.name))
 	}
 }

cli/command/swarm/join_test.go

@@ -23,6 +23,7 @@ func TestSwarmJoinErrors(t *testing.T) {
 	}{
 		{
 			name:          "not-enough-args",
+			args:          []string{},
 			expectedError: "requires 1 argument",
 		},
 		{

cli/command/swarm/join_token.go

@@ -41,7 +41,7 @@ func newJoinTokenCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func runJoinToken(ctx context.Context, dockerCli command.Cli, opts joinTokenOptions) error {
+func runJoinToken(ctx context.Context, dockerCLI command.Cli, opts joinTokenOptions) error {
 	worker := opts.role == "worker"
 	manager := opts.role == "manager"
 
@@ -49,72 +49,71 @@ func runJoinToken(ctx context.Context, dockerCli command.Cli, opts joinTokenOpti
 		return errors.New("unknown role " + opts.role)
 	}
 
-	client := dockerCli.Client()
+	apiClient := dockerCLI.Client()
 
 	if opts.rotate {
-		flags := swarm.UpdateFlags{
+		sw, err := apiClient.SwarmInspect(ctx)
-			RotateWorkerToken:  worker,
-			RotateManagerToken: manager,
-		}
-
-		sw, err := client.SwarmInspect(ctx)
 		if err != nil {
 			return err
 		}
 
-		if err := client.SwarmUpdate(ctx, sw.Version, sw.Spec, flags); err != nil {
+		err = apiClient.SwarmUpdate(ctx, sw.Version, sw.Spec, swarm.UpdateFlags{
+			RotateWorkerToken:  worker,
+			RotateManagerToken: manager,
+		})
+		if err != nil {
 			return err
 		}
 
 		if !opts.quiet {
-			fmt.Fprintf(dockerCli.Out(), "Successfully rotated %s join token.\n\n", opts.role)
+			_, _ = fmt.Fprintf(dockerCLI.Out(), "Successfully rotated %s join token.\n\n", opts.role)
 		}
 	}
 
 	// second SwarmInspect in this function,
 	// this is necessary since SwarmUpdate after first changes the join tokens
-	sw, err := client.SwarmInspect(ctx)
+	sw, err := apiClient.SwarmInspect(ctx)
 	if err != nil {
 		return err
 	}
 
 	if opts.quiet && worker {
-		fmt.Fprintln(dockerCli.Out(), sw.JoinTokens.Worker)
+		_, _ = fmt.Fprintln(dockerCLI.Out(), sw.JoinTokens.Worker)
 		return nil
 	}
 
 	if opts.quiet && manager {
-		fmt.Fprintln(dockerCli.Out(), sw.JoinTokens.Manager)
+		_, _ = fmt.Fprintln(dockerCLI.Out(), sw.JoinTokens.Manager)
 		return nil
 	}
 
-	info, err := client.Info(ctx)
+	info, err := apiClient.Info(ctx)
 	if err != nil {
 		return err
 	}
 
-	return printJoinCommand(ctx, dockerCli, info.Swarm.NodeID, worker, manager)
+	return printJoinCommand(ctx, dockerCLI, info.Swarm.NodeID, worker, manager)
 }
 
-func printJoinCommand(ctx context.Context, dockerCli command.Cli, nodeID string, worker bool, manager bool) error {
+func printJoinCommand(ctx context.Context, dockerCLI command.Cli, nodeID string, worker bool, manager bool) error {
-	client := dockerCli.Client()
+	apiClient := dockerCLI.Client()
 
-	node, _, err := client.NodeInspectWithRaw(ctx, nodeID)
+	node, _, err := apiClient.NodeInspectWithRaw(ctx, nodeID)
 	if err != nil {
 		return err
 	}
 
-	sw, err := client.SwarmInspect(ctx)
+	sw, err := apiClient.SwarmInspect(ctx)
 	if err != nil {
 		return err
 	}
 
 	if node.ManagerStatus != nil {
 		if worker {
-			fmt.Fprintf(dockerCli.Out(), "To add a worker to this swarm, run the following command:\n\n    docker swarm join --token %s %s\n\n", sw.JoinTokens.Worker, node.ManagerStatus.Addr)
+			_, _ = fmt.Fprintf(dockerCLI.Out(), "To add a worker to this swarm, run the following command:\n\n    docker swarm join --token %s %s\n\n", sw.JoinTokens.Worker, node.ManagerStatus.Addr)
 		}
 		if manager {
-			fmt.Fprintf(dockerCli.Out(), "To add a manager to this swarm, run the following command:\n\n    docker swarm join --token %s %s\n\n", sw.JoinTokens.Manager, node.ManagerStatus.Addr)
+			_, _ = fmt.Fprintf(dockerCLI.Out(), "To add a manager to this swarm, run the following command:\n\n    docker swarm join --token %s %s\n\n", sw.JoinTokens.Manager, node.ManagerStatus.Addr)
 		}
 	}
 

cli/command/swarm/join_token_test.go

@@ -27,6 +27,7 @@ func TestSwarmJoinTokenErrors(t *testing.T) {
 	}{
 		{
 			name:          "not-enough-args",
+			args:          []string{},
 			expectedError: "requires 1 argument",
 		},
 		{

cli/command/swarm/leave_test.go

@@ -25,6 +25,7 @@ func TestSwarmLeaveErrors(t *testing.T) {
 		},
 		{
 			name: "leave-failed",
+			args: []string{},
 			swarmLeaveFunc: func() error {
 				return errors.New("error leaving the swarm")
 			},
@@ -48,6 +49,9 @@ func TestSwarmLeaveErrors(t *testing.T) {
 func TestSwarmLeave(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{})
 	cmd := newLeaveCommand(cli)
+	cmd.SetArgs([]string{})
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
 	assert.NilError(t, cmd.Execute())
 	assert.Check(t, is.Equal("Node left the swarm.", strings.TrimSpace(cli.OutBuffer().String())))
 }

cli/command/swarm/testdata/init-init-autolock.golden

@@ -1,7 +1,6 @@
 Swarm initialized: current node (nodeID) is now a manager.
 
 To add a manager to this swarm, run 'docker swarm join-token manager' and follow the instructions.
-
 To unlock a swarm manager after it restarts, run the `docker swarm unlock`
 command and provide the following key:
 

cli/command/swarm/testdata/init-init.golden

@@ -1,4 +1,3 @@
 Swarm initialized: current node (nodeID) is now a manager.
 
 To add a manager to this swarm, run 'docker swarm join-token manager' and follow the instructions.
-

cli/command/swarm/testdata/unlockkeys-unlock-key-rotate.golden

@@ -1,5 +1,4 @@
 Successfully rotated manager unlock key.
-
 To unlock a swarm manager after it restarts, run the `docker swarm unlock`
 command and provide the following key:
 

cli/command/swarm/unlock_key.go

@@ -42,13 +42,13 @@ func newUnlockKeyCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func runUnlockKey(ctx context.Context, dockerCli command.Cli, opts unlockKeyOptions) error {
+func runUnlockKey(ctx context.Context, dockerCLI command.Cli, opts unlockKeyOptions) error {
-	client := dockerCli.Client()
+	apiClient := dockerCLI.Client()
 
 	if opts.rotate {
 		flags := swarm.UpdateFlags{RotateManagerUnlockKey: true}
 
-		sw, err := client.SwarmInspect(ctx)
+		sw, err := apiClient.SwarmInspect(ctx)
 		if err != nil {
 			return err
 		}
@@ -57,16 +57,16 @@ func runUnlockKey(ctx context.Context, dockerCli command.Cli, opts unlockKeyOpti
 			return errors.New("cannot rotate because autolock is not turned on")
 		}
 
-		if err := client.SwarmUpdate(ctx, sw.Version, sw.Spec, flags); err != nil {
+		if err := apiClient.SwarmUpdate(ctx, sw.Version, sw.Spec, flags); err != nil {
 			return err
 		}
 
 		if !opts.quiet {
-			fmt.Fprintf(dockerCli.Out(), "Successfully rotated manager unlock key.\n\n")
+			_, _ = fmt.Fprintln(dockerCLI.Out(), "Successfully rotated manager unlock key.")
 		}
 	}
 
-	unlockKeyResp, err := client.SwarmGetUnlockKey(ctx)
+	unlockKeyResp, err := apiClient.SwarmGetUnlockKey(ctx)
 	if err != nil {
 		return errors.Wrap(err, "could not fetch unlock key")
 	}
@@ -76,17 +76,17 @@ func runUnlockKey(ctx context.Context, dockerCli command.Cli, opts unlockKeyOpti
 	}
 
 	if opts.quiet {
-		fmt.Fprintln(dockerCli.Out(), unlockKeyResp.UnlockKey)
+		_, _ = fmt.Fprintln(dockerCLI.Out(), unlockKeyResp.UnlockKey)
 		return nil
 	}
 
-	printUnlockCommand(dockerCli.Out(), unlockKeyResp.UnlockKey)
+	printUnlockCommand(dockerCLI.Out(), unlockKeyResp.UnlockKey)
 	return nil
 }
 
 func printUnlockCommand(out io.Writer, unlockKey string) {
 	if len(unlockKey) > 0 {
-		fmt.Fprintf(out, "To unlock a swarm manager after it restarts, "+
+		_, _ = fmt.Fprintf(out, "To unlock a swarm manager after it restarts, "+
 			"run the `docker swarm unlock`\ncommand and provide the following key:\n\n    %s\n\n"+
 			"Remember to store this key in a password manager, since without it you\n"+
 			"will not be able to restart the manager.\n", unlockKey)

cli/command/swarm/unlock_key_test.go

@@ -87,12 +87,16 @@ func TestSwarmUnlockKeyErrors(t *testing.T) {
 					swarmUpdateFunc:       tc.swarmUpdateFunc,
 					swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc,
 				}))
-			cmd.SetArgs(tc.args)
+			if tc.args == nil {
-			for k, v := range tc.flags {
+				cmd.SetArgs([]string{})
-				assert.Check(t, cmd.Flags().Set(k, v))
+			} else {
+				cmd.SetArgs(tc.args)
 			}
 			cmd.SetOut(io.Discard)
 			cmd.SetErr(io.Discard)
+			for k, v := range tc.flags {
+				assert.Check(t, cmd.Flags().Set(k, v))
+			}
 			assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 		})
 	}
@@ -101,7 +105,6 @@ func TestSwarmUnlockKeyErrors(t *testing.T) {
 func TestSwarmUnlockKey(t *testing.T) {
 	testCases := []struct {
 		name                  string
-		args                  []string
 		flags                 map[string]string
 		swarmInspectFunc      func() (swarm.Swarm, error)
 		swarmUpdateFunc       func(swarm swarm.Spec, flags swarm.UpdateFlags) error
@@ -164,7 +167,9 @@ func TestSwarmUnlockKey(t *testing.T) {
 				swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc,
 			})
 			cmd := newUnlockKeyCommand(cli)
-			cmd.SetArgs(tc.args)
+			cmd.SetArgs([]string{})
+			cmd.SetOut(io.Discard)
+			cmd.SetErr(io.Discard)
 			for k, v := range tc.flags {
 				assert.Check(t, cmd.Flags().Set(k, v))
 			}

cli/command/swarm/unlock_test.go

@@ -70,7 +70,11 @@ func TestSwarmUnlockErrors(t *testing.T) {
 					infoFunc:        tc.infoFunc,
 					swarmUnlockFunc: tc.swarmUnlockFunc,
 				}))
-			cmd.SetArgs(tc.args)
+			if tc.args == nil {
+				cmd.SetArgs([]string{})
+			} else {
+				cmd.SetArgs(tc.args)
+			}
 			cmd.SetOut(io.Discard)
 			cmd.SetErr(io.Discard)
 			assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
@@ -97,5 +101,8 @@ func TestSwarmUnlock(t *testing.T) {
 	})
 	dockerCli.SetIn(streams.NewIn(io.NopCloser(strings.NewReader(input))))
 	cmd := newUnlockCommand(dockerCli)
+	cmd.SetArgs([]string{})
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
 	assert.NilError(t, cmd.Execute())
 }

cli/command/swarm/update_test.go

@@ -72,12 +72,16 @@ func TestSwarmUpdateErrors(t *testing.T) {
 					swarmUpdateFunc:       tc.swarmUpdateFunc,
 					swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc,
 				}))
-			cmd.SetArgs(tc.args)
+			if tc.args == nil {
-			for key, value := range tc.flags {
+				cmd.SetArgs([]string{})
-				assert.Check(t, cmd.Flags().Set(key, value))
+			} else {
+				cmd.SetArgs(tc.args)
 			}
 			cmd.SetOut(io.Discard)
 			cmd.SetErr(io.Discard)
+			for k, v := range tc.flags {
+				assert.Check(t, cmd.Flags().Set(k, v))
+			}
 			assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 		})
 	}
@@ -180,8 +184,8 @@ func TestSwarmUpdate(t *testing.T) {
 			} else {
 				cmd.SetArgs(tc.args)
 			}
-			for key, value := range tc.flags {
+			for k, v := range tc.flags {
-				assert.Check(t, cmd.Flags().Set(key, value))
+				assert.Check(t, cmd.Flags().Set(k, v))
 			}
 			cmd.SetOut(cli.OutBuffer())
 			assert.NilError(t, cmd.Execute())