From 27a98f292040214821fa11a763951d84b76d6f1b Mon Sep 17 00:00:00 2001 From: Thibault Jamet Date: Mon, 4 Mar 2019 09:59:38 +0100 Subject: [PATCH] Fix race condition in metric process collector test There was a goroutine started to log things upon a test that could be ended at the time `cmd.Wait()` ends. To solve the problem, when the sub-test ends, ensure we wait until the command ends when ending the test The output of `make test` before the fix shows: ``` === RUN TestNewUDPLogListener ================== WARNING: DATA RACE Read at 0x00c0002a8643 by goroutine 74: testing.(*common).logDepth() /usr/local/go/src/testing/testing.go:629 +0x132 testing.(*common).Logf() /usr/local/go/src/testing/testing.go:614 +0x90 k8s.io/ingress-nginx/internal/ingress/metric/collectors.TestProcessCollector.func1.1() /go/src/k8s.io/ingress-nginx/internal/ingress/metric/collectors/process_test.go:54 +0x140 Previous write at 0x00c0002a8643 by goroutine 72: testing.tRunner.func1() /usr/local/go/src/testing/testing.go:856 +0x33e testing.tRunner() /usr/local/go/src/testing/testing.go:869 +0x17f Goroutine 74 (running) created at: k8s.io/ingress-nginx/internal/ingress/metric/collectors.TestProcessCollector.func1() /go/src/k8s.io/ingress-nginx/internal/ingress/metric/collectors/process_test.go:50 +0x218 testing.tRunner() /usr/local/go/src/testing/testing.go:865 +0x163 Goroutine 72 (finished) created at: testing.(*T).Run() /usr/local/go/src/testing/testing.go:916 +0x699 testing.runTests.func1() /usr/local/go/src/testing/testing.go:1157 +0xa8 testing.tRunner() /usr/local/go/src/testing/testing.go:865 +0x163 testing.runTests() /usr/local/go/src/testing/testing.go:1155 +0x523 testing.(*M).Run() /usr/local/go/src/testing/testing.go:1072 +0x2eb main.main() _testmain.go:52 +0x222 ================== --- PASS: TestNewUDPLogListener (0.00s) ``` after the patch: ``` === RUN TestNewUDPLogListener --- PASS: TestNewUDPLogListener (0.01s) ``` Change-Id: I8ea246d14f5f80b330be19dd5b8299c6762f6d6b --- internal/ingress/metric/collectors/process_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/ingress/metric/collectors/process_test.go b/internal/ingress/metric/collectors/process_test.go index a460b2b96..45170572b 100644 --- a/internal/ingress/metric/collectors/process_test.go +++ b/internal/ingress/metric/collectors/process_test.go @@ -55,11 +55,13 @@ func TestProcessCollector(t *testing.T) { } else { t.Logf("Status: %v", status.ExitStatus()) } + done <- struct{}{} }() cm, err := NewNGINXProcess("pod", "default", "nginx") if err != nil { t.Errorf("unexpected error creating nginx status collector: %v", err) + t.FailNow() } go cm.Start() @@ -68,6 +70,7 @@ func TestProcessCollector(t *testing.T) { cm.Stop() cmd.Process.Kill() + <-done close(done) }()