cmd/go: TestCoverageRuns fails

[josharian]

Broken out from #17472.

$ go test -run=TestCoverageRuns cmd/go
--- FAIL: TestCoverageRuns (1.41s)
	go_test.go:251: running testgo [test -short -coverpkg=strings strings regexp]
	go_test.go:266: standard output:
	go_test.go:267: --- FAIL: TestIndexRune (0.00s)
			strings_test.go:298: expected no allocations, got 1.000000
		FAIL
		coverage: 97.9% of statements in strings
		FAIL	strings	0.082s
		ok  	regexp	0.099s	coverage: 19.2% of statements in strings
		
	go_test.go:280: go [test -short -coverpkg=strings strings regexp] failed unexpectedly: exit status 1
FAIL
FAIL	cmd/go	3.513s

Looks like the failure is because the coverage instrumentation causes an allocation in TestIndexRune where there wasn't one before, causing the test itself to fail.

[bradfitz]

So IndexRune is allocation-free, except when it's compiled in cover mode?

We could just skip that alloc test in that mode, but how do we detect that we're in cover mode at runtime?

/cc @dsnet

[josharian]

Yep. Alternatively, we could have TestCoverageRuns skip the test, but (frustratingly) there's no way to tell go test to skip tests that match a regexp.

[dsnet]

My understanding is that coverage instruments the code and inserts logic to do some bookkeeping. What is it doing that's causing the allocation? (I can look into this).

I see at least 50 usages of testing.AllocsPerRun in stdlib. Do those also fail when we run them with coverage enabled or is IndexRune just unlucky?

I'm wondering if there's a bigger problem where it is flaky to do AllocsPerRun when in coverage mode.

[bradfitz]

If only AllocsPerRun took a *testing.T, then it could just always t.Skip in coverage mode. :)

[josharian]

I'm wondering if there's a bigger problem where it is flaky to do AllocsPerRun when in coverage mode.

Probably. Unfortunately, pprof doesn't help much here. Running 'go test -run=TestIndexRune -memprofile=m -coverprofile=c strings' yields line numbers that are useless, because they're post-cover-rewrite line numbers. But there are four sources of allocations there, somewhere, so the complaint from AllocsPerRun is legit.

[josharian]

If only AllocsPerRun took a *testing.T, then it could just always t.Skip in coverage mode. :)

And if *testing.T exposed whether it was in coverage mode.

[bradfitz]

/cc @robpike

[gopherbot]

CL https://golang.org/cl/32483 mentions this issue.

[gopherbot]

CL https://golang.org/cl/32484 mentions this issue.