cli/rm_test: Fix TestRemoveForce race condition

Synchronize append on the `removed` slice with mutex because
containerRemoveFunc is called in parallel for each removed container by
`container rm` cli command.
Also reduced the shared access area by separating the scopes of test
cases.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit b811057181)
This commit is contained in:
Paweł Gronowski 2023-01-04 10:47:46 +01:00
parent 9889fa575a
commit b309569bc6
No known key found for this signature in database
GPG Key ID: B85EFCFE26DEF92A
1 changed files with 41 additions and 31 deletions

View File

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