Wait longer for a stable goroutine count in tests#49017
Merged
thaJeztah merged 2 commits intomoby:masterfrom Dec 3, 2024
Merged
Wait longer for a stable goroutine count in tests#49017thaJeztah merged 2 commits intomoby:masterfrom
thaJeztah merged 2 commits intomoby:masterfrom
Conversation
robmry
added
kind/enhancement
Signed-off-by: Rob Murray <[email protected]>
Test 'TestDockerCLIRunSuite/TestRunAttachFailedNoLeak' does this ...
- start a container that exits immediately, its comment says:
- "Run a dummy container to ensure all goroutines are up and running
before we get a count"
- wait for the number of goroutines to be stable for 400ms, and remember
that number
- start a container
- start another container, expecting it to fail with a port-mapping clash
- stop the running container
- wait for up to 30s for the number of goroutines to fall back to the
remembered number.
In a CI run - hacking in some debug to count goroutines once a second,
before waiting for the number to stablilise for 400ms, showed that the
initial (dummy) container run had no immediate effect. But, three more
goroutines appeared within a few seconds. For example:
=== RUN TestDockerCLIRunSuite/TestRunAttachFailedNoLeak
docker_cli_run_test.go:3822: goroutines before container run: 47 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 47 i 0 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 48 i 1 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 48 i 2 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 48 i 3 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 48 i 4 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 5 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 6 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 7 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 8 err <nil>
docker_cli_run_test.go:3830: goroutines after container run: 50 i 9 err <nil>
That means a delay while running the rest of the test risks finding the
extra goroutines that are going to start anyway and not go away (regardless
of whether more containers are started).
So - wait for the goroutine count to be stable for 7s, rather than 400ms.
Signed-off-by: Rob Murray <[email protected]>
robmry
force-pushed
the
stable_goroutine_count
branch
from
a94962f to
d75394b
Compare
laurazard
approved these changes
Dec 3, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- What I did
Test 'TestDockerCLIRunSuite/TestRunAttachFailedNoLeak' does this ...
In a CI run - hacking in some debug to count goroutines once a second, before waiting for the number to stablilise for 400ms, showed that the initial (dummy) container run had no immediate effect. But, three more goroutines appeared within a few seconds. For example:
That means a delay while running the rest of the test risks finding the extra goroutines that are going to start anyway and not go away (regardless of whether more containers are started).
- How I did it
So - wait for the goroutine count to be stable for 7s, rather than 400ms.
- How to verify it
CI
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)