cmd/vet: check for concurrent use of T.FatalX and T.SkipX

[dsnet]

The testing package is explicit about the following:

A test ends when its Test function returns or calls any of the methods FailNow, Fatal, Fatalf, SkipNow, Skip, or Skipf. Those methods, as well as the Parallel method, must be called only from the goroutine running the Test function.

However, this is easy to overlook and it is not immediately obvious that the code below is a bad test:

func TestFoo(t *testing.T) {
    go func() {
        t.Fatal("fatal") // Called from outside the Test function
    }()
    time.Sleep(1 * time.Second)
}

This outputs (what a user expects):

--- FAIL: TestFoo (1.00s)
    foo_test.go:10: fatal

Giving the user a false sense of having written correct test.

However, when running the test with the -race flag, it becomes obvious this is not okay:

==================
WARNING: DATA RACE
....
==================
--- FAIL: TestFoo (1.00s)
    foo_test.go:10: fatal

Perhaps the vet tool can check for these cases.

[josharian]

I think if the race detector catches the problem then vet doesn't need to check too.

[dsnet]

The difficulty is that the Fatal case isn't always triggered, so the race detector isn't able to detect them. But when Fatal is actually hit, funky things can sometimes happen.

Go vet offers the ability to detect these abuses of the testing package earlier on.

It comes down to static analysis vs dynamic analysis.