Skip to content

cli/rm_test: Fix TestRemoveForce race condition - #3943

Merged
thaJeztah merged 1 commit into
docker:masterfrom
vvoland:fix-test-removeforce
Jan 6, 2023
Merged

cli/rm_test: Fix TestRemoveForce race condition#3943
thaJeztah merged 1 commit into
docker:masterfrom
vvoland:fix-test-removeforce

Conversation

@vvoland

@vvolandvvoland commented Jan 4, 2023

Copy link
Copy Markdown
Collaborator

There was already an attempt to fix this test by #3520 but it seems that it only accounted for the races between the test cases, and not the container rm itself calling containerRemoveFunc concurrently.

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

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>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #3943 (b811057) into master (850fb69) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@ Coverage Diff @@## master #3943 +/- ##
=======================================
Coverage 59.20% 59.20% =======================================
Files 287 287 Lines 24687 24687 =======================================
Hits 14617 14617 Misses 9186 9186 Partials 884 884 

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@thaJeztah
thaJeztah merged commit e5b29b8 into docker:masterJan 6, 2023
@thaJeztahthaJeztah added this to the 23.0.0 milestone Jan 6, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestRemoveForce may panic when testing with parallel

3 participants

@vvoland@codecov-commenter@thaJeztah