Merge pull request #3943 from vvoland/fix-test-removeforce

cli/rm_test: Fix TestRemoveForce race condition

cli/command/container/rm_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"sort"
+	"sync"
 	"testing"
 
 	"github.com/docker/cli/internal/test"
@@ -14,37 +15,46 @@ import (
 )
 
 func TestRemoveForce(t *testing.T) {
-	var (
-		removed1 []string
-		removed2 []string
-	)
+	for _, tc := range []struct {
+		name        string
+		args        []string
+		expectedErr string
+	}{
+		{name: "without force", args: []string{"nosuchcontainer", "mycontainer"}, expectedErr: "no such container"},
+		{name: "with force", args: []string{"--force", "nosuchcontainer", "mycontainer"}, expectedErr: ""},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			var removed []string
+			mutex := new(sync.Mutex)
 
-	cli := test.NewFakeCli(&fakeClient{
-		containerRemoveFunc: func(ctx context.Context, container string, options types.ContainerRemoveOptions) error {
-			removed1 = append(removed1, container)
-			removed2 = append(removed2, container)
-			if container == "nosuchcontainer" {
-				return errdefs.NotFound(fmt.Errorf("Error: no such container: " + container))
+			cli := test.NewFakeCli(&fakeClient{
+				containerRemoveFunc: func(ctx context.Context, container string, options types.ContainerRemoveOptions) error {
+					// containerRemoveFunc is called in parallel for each container
+					// by the remove command so append must be synchronized.
+					mutex.Lock()
+					removed = append(removed, container)
+					mutex.Unlock()
+
+					if container == "nosuchcontainer" {
+						return errdefs.NotFound(fmt.Errorf("Error: no such container: " + container))
+					}
+					return nil
+				},
+				Version: "1.36",
+			})
+			cmd := NewRmCommand(cli)
+			cmd.SetOut(io.Discard)
+			cmd.SetArgs(tc.args)
+
+			err := cmd.Execute()
+			if tc.expectedErr != "" {
+				assert.ErrorContains(t, err, tc.expectedErr)
+			} else {
+				assert.NilError(t, err)
 			}
-			return nil
-		},
-		Version: "1.36",
-	})
-	cmd := NewRmCommand(cli)
-	cmd.SetOut(io.Discard)
-
-	t.Run("without force", func(t *testing.T) {
-		cmd.SetArgs([]string{"nosuchcontainer", "mycontainer"})
-		removed1 = []string{}
-		assert.ErrorContains(t, cmd.Execute(), "no such container")
-		sort.Strings(removed1)
-		assert.DeepEqual(t, removed1, []string{"mycontainer", "nosuchcontainer"})
-	})
-	t.Run("with force", func(t *testing.T) {
-		cmd.SetArgs([]string{"--force", "nosuchcontainer", "mycontainer"})
-		removed2 = []string{}
-		assert.NilError(t, cmd.Execute())
-		sort.Strings(removed2)
-		assert.DeepEqual(t, removed2, []string{"mycontainer", "nosuchcontainer"})
-	})
+			sort.Strings(removed)
+			assert.DeepEqual(t, removed, []string{"mycontainer", "nosuchcontainer"})
+		})
+	}
 }