Skip to content

awslogs: fix flaky TestLogBlocking unit test#39968

Merged
kolyshkin merged 1 commit intomoby:masterfrom
samuelkarp:issue-39857
Sep 23, 2019
Merged

awslogs: fix flaky TestLogBlocking unit test#39968
kolyshkin merged 1 commit intomoby:masterfrom
samuelkarp:issue-39857

Conversation

@samuelkarp
Copy link
Member

- What I did
Fix #39857

- How I did it
TestLogBlocking is intended to test that the Log method blocks by default. It does this by mocking out the internals of the awslogs.logStream and replacing one of its internal channels with one that is controlled by the test. The call to Log occurs inside a goroutine. Go may or may not schedule the goroutine immediately and the blocking may or may not be observed outside the goroutine immediately due to decisions made by the Go runtime. This change adds a small timeout for test failure so that the Go runtime has the opportunity to run the goroutine before the test fails.

- How to verify it
TestLogBlocking doesn't fail anymore.

$ go test -run TestLogBlocking  -count 10000 ./daemon/logger/awslogs/
ok  	github.com/docker/docker/daemon/logger/awslogs	0.476s

- Description for the changelog
N/A

TestLogBlocking is intended to test that the Log method blocks by
default.  It does this by mocking out the internals of the
awslogs.logStream and replacing one of its internal channels with one
that is controlled by the test.  The call to Log occurs inside a
goroutine.  Go may or may not schedule the goroutine immediately and the
blocking may or may not be observed outside the goroutine immediately
due to decisions made by the Go runtime.  This change adds a small
timeout for test failure so that the Go runtime has the opportunity to
run the goroutine before the test fails.

Signed-off-by: Samuel Karp <skarp@amazon.com>
@samuelkarp
Copy link
Member Author

I'm not sure what failed, but in the failed run I still see this:

[2019-09-20T23:29:14.035Z] ok  	github.com/docker/docker/daemon/logger/awslogs	0.635s	coverage: 77.8% of statements

@thaJeztah
Copy link
Member

thank you! I kicked CI to run again

@thaJeztah
Copy link
Member

@ddebroy @vikramhh @kolyshkin PTAL

@vikramhh
Copy link

LGTM

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

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Flaky test: awslogs tests failing with ResourceAlreadyExistsException

4 participants