Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[20.10 backport] integration-cli: Fix hanging TestLogsFollowGoroutines* #44765

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Jan 9, 2023

- What I did
Fix TestLogsFollowGoroutinesWithStdout and TestLogsFollowGoroutinesNoOutput hanging randomly

- How I did it
cmd.Wait is called twice from different goroutines which can cause the test to hang completely. Fix by calling Wait only once and sending its return value over a channel.

In TestLogsFollowGoroutinesWithStdout also added additional closes and process kills to ensure that we don't leak anything in case test returns early because of failed test assertion.

- How to verify it

make TEST_FILTER='.*TestLogsFollowGoroutine.*' TESTFLAGS='-test.count 10000 '  test-integration

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

cmd.Wait is called twice from different goroutines which can cause the
test to hang completely. Fix by calling Wait only once and sending its
return value over a channel.

In TestLogsFollowGoroutinesWithStdout also added additional closes and
process kills to ensure that we don't leak anything in case test returns
early because of failed test assertion.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit deb4910)
@vvoland vvoland added area/testing kind/bugfix PR's that fix bugs labels Jan 9, 2023
@vvoland vvoland added this to the 20.10.23 milestone Jan 9, 2023
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

yay, green! (was hitting the jackpot on flaky tests on this PR 😞)

@thaJeztah thaJeztah merged commit 864cc1c into moby:20.10 Jan 10, 2023
Sign up for free to 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.

None yet

2 participants