[test] Call cmd.Start and cmd.Wait separately to avoid triggering race detector

[jeromefroe]

Hi!

I noticed that currently the tests in node_exporter.go fail when run with the race detector. For example, on master:

$ go test -race -v
=== RUN   TestFileDescriptorLeak
--- SKIP: TestFileDescriptorLeak (0.00s)
	node_exporter_test.go:26: proc filesystem is not available, but currently required to read number of open file descriptors: could not read /proc: stat /proc: no such file or directory
=== RUN   TestHandlingOfDuplicatedMetrics
==================
WARNING: DATA RACE
Read at 0x00c4201960a0 by goroutine 12:
  github.com/prometheus/node_exporter.runCommandAndTests()
      /Users/jeromefroelich/golang/src/github.com/prometheus/node_exporter/node_exporter_test.go:128 +0x17b
  github.com/prometheus/node_exporter.TestHandlingOfDuplicatedMetrics()
      /Users/jeromefroelich/golang/src/github.com/prometheus/node_exporter/node_exporter_test.go:86 +0x631
  testing.tRunner()
      /usr/local/opt/go/libexec/src/testing/testing.go:657 +0x107

Previous write at 0x00c4201960a0 by goroutine 13:
  os/exec.(*Cmd).Start()
      /usr/local/opt/go/libexec/src/os/exec/exec.go:354 +0x78c
  os/exec.(*Cmd).Run()
      /usr/local/opt/go/libexec/src/os/exec/exec.go:277 +0x3c
  github.com/prometheus/node_exporter.runCommandAndTests.func1()
      /Users/jeromefroelich/golang/src/github.com/prometheus/node_exporter/node_exporter_test.go:112 +0x50

Goroutine 12 (running) created at:
  testing.(*T).Run()
      /usr/local/opt/go/libexec/src/testing/testing.go:697 +0x543
  testing.runTests.func1()
      /usr/local/opt/go/libexec/src/testing/testing.go:882 +0xaa
  testing.tRunner()
      /usr/local/opt/go/libexec/src/testing/testing.go:657 +0x107
  testing.runTests()
      /usr/local/opt/go/libexec/src/testing/testing.go:888 +0x4e0
  testing.(*M).Run()
      /usr/local/opt/go/libexec/src/testing/testing.go:822 +0x1c3
  main.main()
      github.com/prometheus/node_exporter/_test/_testmain.go:44 +0x20f

Goroutine 13 (running) created at:
  github.com/prometheus/node_exporter.runCommandAndTests()
      /Users/jeromefroelich/golang/src/github.com/prometheus/node_exporter/node_exporter_test.go:117 +0x8f
  github.com/prometheus/node_exporter.TestHandlingOfDuplicatedMetrics()
      /Users/jeromefroelich/golang/src/github.com/prometheus/node_exporter/node_exporter_test.go:86 +0x631
  testing.tRunner()
      /usr/local/opt/go/libexec/src/testing/testing.go:657 +0x107**
...

The issue is that runCommandAndTests calls cmd.Run in a separate goroutine from the one in which it gets the pid for the process by calling cmd.Process.Pid. This causes the race detector to trigger because it sees an unsynchronized write and read to the same variable in separate goroutines. cmd.Run calls cmd.Start, which updates the Process field for cmd, and then waits for the cmd.Wait to return. Consequently, we can avoid triggering the race detector by calling cmd.Start ourselves and then calling cmd.Wait in a separate goroutine. This fixes the race because now the write and read of cmd.Process occur in the same goroutine. For example, on my branch:

$ go test -race -v
=== RUN   TestFileDescriptorLeak
--- SKIP: TestFileDescriptorLeak (0.00s)
	node_exporter_test.go:26: proc filesystem is not available, but currently required to read number of open file descriptors: could not read /proc: stat /proc: no such file or directory
=== RUN   TestHandlingOfDuplicatedMetrics
--- PASS: TestHandlingOfDuplicatedMetrics (0.23s)
PASS
ok  	github.com/prometheus/node_exporter	1.293s

[SuperQ]

Want to add -test to the Makefile as well?

[jeromefroe]

Hey, yep, just pushed a change to enable the race detector during tests in the Makefile.

[SuperQ]

LGTM