From fd94bae0b8acc451a79667dd1cef4c583d532187 Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Fri, 20 Sep 2019 16:02:22 -0700 Subject: [PATCH] awslogs: fix flaky TestLogBlocking unit test 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 --- daemon/logger/awslogs/cloudwatchlogs_test.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/daemon/logger/awslogs/cloudwatchlogs_test.go b/daemon/logger/awslogs/cloudwatchlogs_test.go index 935e7038cd..420f4f6562 100644 --- a/daemon/logger/awslogs/cloudwatchlogs_test.go +++ b/daemon/logger/awslogs/cloudwatchlogs_test.go @@ -24,7 +24,6 @@ import ( "github.com/docker/docker/dockerversion" "gotest.tools/assert" is "gotest.tools/assert/cmp" - "gotest.tools/skip" ) const ( @@ -286,8 +285,10 @@ func TestLogClosed(t *testing.T) { } } +// TestLogBlocking tests that the Log method blocks appropriately when +// non-blocking behavior is not enabled. Blocking is achieved through an +// internal channel that must be drained for Log to return. func TestLogBlocking(t *testing.T) { - skip.If(t, runtime.GOOS == "windows", "FIXME: test is flaky on Windows. See #39857") mockClient := newMockClient() stream := &logStream{ client: mockClient, @@ -301,18 +302,20 @@ func TestLogBlocking(t *testing.T) { err := stream.Log(&logger.Message{}) errorCh <- err }() + // block until the goroutine above has started <-started select { case err := <-errorCh: t.Fatal("Expected stream.Log to block: ", err) default: - break } + // assuming it is blocked, we can now try to drain the internal channel and + // unblock it select { - case <-stream.messages: - break - default: + case <-time.After(10 * time.Millisecond): + // if we're unable to drain the channel within 10ms, something seems broken t.Fatal("Expected to be able to read from stream.messages but was unable to") + case <-stream.messages: } select { case err := <-errorCh: