Browse code

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 <skarp@amazon.com>
(cherry picked from commit fd94bae0b8acc451a79667dd1cef4c583d532187)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Samuel Karp authored on 2019/09/21 08:02:22
Showing 1 changed files
... ...
@@ -24,7 +24,6 @@ import (
24 24
 	"github.com/docker/docker/dockerversion"
25 25
 	"gotest.tools/assert"
26 26
 	is "gotest.tools/assert/cmp"
27
-	"gotest.tools/skip"
28 27
 )
29 28
 
30 29
 const (
... ...
@@ -286,8 +285,10 @@ func TestLogClosed(t *testing.T) {
286 286
 	}
287 287
 }
288 288
 
289
+// TestLogBlocking tests that the Log method blocks appropriately when
290
+// non-blocking behavior is not enabled.  Blocking is achieved through an
291
+// internal channel that must be drained for Log to return.
289 292
 func TestLogBlocking(t *testing.T) {
290
-	skip.If(t, runtime.GOOS == "windows", "FIXME: test is flaky on Windows. See #39857")
291 293
 	mockClient := newMockClient()
292 294
 	stream := &logStream{
293 295
 		client:   mockClient,
... ...
@@ -301,18 +302,20 @@ func TestLogBlocking(t *testing.T) {
301 301
 		err := stream.Log(&logger.Message{})
302 302
 		errorCh <- err
303 303
 	}()
304
+	// block until the goroutine above has started
304 305
 	<-started
305 306
 	select {
306 307
 	case err := <-errorCh:
307 308
 		t.Fatal("Expected stream.Log to block: ", err)
308 309
 	default:
309
-		break
310 310
 	}
311
+	// assuming it is blocked, we can now try to drain the internal channel and
312
+	// unblock it
311 313
 	select {
312
-	case <-stream.messages:
313
-		break
314
-	default:
314
+	case <-time.After(10 * time.Millisecond):
315
+		// if we're unable to drain the channel within 10ms, something seems broken
315 316
 		t.Fatal("Expected to be able to read from stream.messages but was unable to")
317
+	case <-stream.messages:
316 318
 	}
317 319
 	select {
318 320
 	case err := <-errorCh: