Skip to content

Commit

Permalink
fix race condition in TestRemoveForce
Browse files Browse the repository at this point in the history
This test uses two subtests that were sharing the same variable.
Subtests run in a goroutine, which could lead to them concurrently
accessing the variable, resulting in a panic:

    === FAIL: cli/command/container TestRemoveForce/without_force (0.00s)
    Error: Error: No such container: nosuchcontainer
        --- FAIL: TestRemoveForce/without_force (0.00s)
    panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    	panic: runtime error: invalid memory address or nil pointer dereference
    [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x40393f]
    goroutine 190 [running]:
    testing.tRunner.func1.2({0xb76380, 0x124c9a0})
    	/usr/local/go/src/testing/testing.go:1389 +0x24e
    testing.tRunner.func1()
    	/usr/local/go/src/testing/testing.go:1392 +0x39f
    panic({0xb76380, 0x124c9a0})
    	/usr/local/go/src/runtime/panic.go:838 +0x207
    sort.StringSlice.Less(...)
    	/usr/local/go/src/sort/sort.go:319
    sort.insertionSort({0xd87380, 0xc00051b3b0}, 0x0, 0x2)
    	/usr/local/go/src/sort/sort.go:40 +0xb1
    sort.quickSort({0xd87380, 0xc00051b3b0}, 0x18?, 0xb4f060?, 0xc000540e01?)
    	/usr/local/go/src/sort/sort.go:222 +0x171
    sort.Sort({0xd87380, 0xc00051b3b0})
    	/usr/local/go/src/sort/sort.go:231 +0x53
    sort.Strings(...)
    	/usr/local/go/src/sort/sort.go:335
    github.com/docker/cli/cli/command/container.TestRemoveForce.func2(0xc0005389c0?)
    	/go/src/github.com/docker/cli/cli/command/container/rm_test.go:36 +0x125
    testing.tRunner(0xc00053e4e0, 0xc00051b140)
    	/usr/local/go/src/testing/testing.go:1439 +0x102
    created by testing.(*T).Run
    	/usr/local/go/src/testing/testing.go:1486 +0x35f
    === FAIL: cli/command/container TestRemoveForce (0.00s)

This patch changes the test to use to separate variables.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9688f62)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed Aug 26, 2022
1 parent 0785bd7 commit 6b25bc3
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions cli/command/container/rm_test.go
Expand Up @@ -14,13 +14,17 @@ import (
)

func TestRemoveForce(t *testing.T) {
var removed []string
var (
removed1 []string
removed2 []string
)

cli := test.NewFakeCli(&fakeClient{
containerRemoveFunc: func(ctx context.Context, container string, options types.ContainerRemoveOptions) error {
removed = append(removed, container)
removed1 = append(removed1, container)
removed2 = append(removed2, container)
if container == "nosuchcontainer" {
return errdefs.NotFound(fmt.Errorf("Error: No such container: " + container))
return errdefs.NotFound(fmt.Errorf("Error: no such container: " + container))
}
return nil
},
Expand All @@ -31,16 +35,16 @@ func TestRemoveForce(t *testing.T) {

t.Run("without force", func(t *testing.T) {
cmd.SetArgs([]string{"nosuchcontainer", "mycontainer"})
removed = []string{}
assert.ErrorContains(t, cmd.Execute(), "No such container")
sort.Strings(removed)
assert.DeepEqual(t, removed, []string{"mycontainer", "nosuchcontainer"})
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"})
removed = []string{}
removed2 = []string{}
assert.NilError(t, cmd.Execute())
sort.Strings(removed)
assert.DeepEqual(t, removed, []string{"mycontainer", "nosuchcontainer"})
sort.Strings(removed2)
assert.DeepEqual(t, removed2, []string{"mycontainer", "nosuchcontainer"})
})
}

0 comments on commit 6b25bc3

Please sign in to comment.