proposal: testing: add a way to provide additional coverage information

[rogpeppe]

The current test coverage mechanism works well when the test binary runs only once with all the tests running within that process, but sometimes it is useful to be able to run the test binary itself within the tests. For example, we may be testing a command line program that has global variables and we want to avoid inter-test pollution. The go tool uses this kind of approach, for example, by compiling itself, but it could potentially also use its own test binary.. Another example is when some code we might wish to exercise is in the form of a top level "main" function. For example, see analysis/singlechecker.

By default, although coverage information is extremely useful, it's not easy to obtain when a test suite runs the test binary as part of its tests. It is just about possible (this package does it), but only by relying on fragile parts of the system that may change in the future.

To solve this issue, I propose two new mechanisms:

1. a way to tell the testing package to read a coverage file and add it to the total coverage
2. a way to ask the testing package to write coverage report information even when M.Run is not called.

A possible API might look like this:

package testing

// AddCoverage adds coverage information from the given file to the total test coverage.
// The file must have been produced by the current test binary. This can
// be used to add additional coverage information written by M.WriteCoverage.
// This is a no-op if the binary was not built with coverage information enabled.
func AddCoverage(coverageFile string) error

// WriteCoverage writes coverage information to the given file.
// This is usually done automatically, but is useful if the test
// binary is re-executed as part of testing.
// This is a no-op if the binary was not built with coverage information enabled.
func (m *testing.M) WriteCoverage(file string) error

See also issue #30231

[mvdan]

/cc @alandonovan

[alandonovan]

I would prefer to decouple this mechanism from the testing package, because the need to call WriteCoverage explicitly seems to arise particularly within standalone programs used as part of a test that are not the test executable itself. With the Go build system, one tends to fork/exec os.Args[0] using the same binary but a different entry point, but with Bazel/Blaze, it's easy for one program (the test) to depend on a completely different program (the helper), where the code subject to coverage is in the helper, and the helper doesn't depend on testing. Perhaps we split testing/cover.go into a separate "cover" package and expose its Register and Report functions?

FWIW, Blaze provides coverage through a different pipeline and file format (LCOV). The structure is similar but the details vary. There's a package called coverdata that lives at the very bottom of the dependency graph, and which acts as a register of all coverage counters, and it exposes functions to register counters for a file and to write a report. There are on the order of a dozen calls to the report function from helper programs. (Many of those are made indirectly through a mechanism for deferred cleanup calls similar to C's atexit(3).) The implementation of WriteReport for Go in Blaze would have to be patched to write in the LCOV format. The need for the patch is no worse that what we have today, but it would at least standardize for users the API by which a helper flushes its coverage report.

@bcmills, @jayconrod to ensure that whatever API we come up with works for Go, Bazel, and Blaze.

[josharian]

Depending on the details, go-fuzz could also benefit from standardization. It currently mimics Go, with some minor incompatibilities. (I am unsure offhand whether those incompatibilities are bugs or due to the fact that go-fuzz gathers some expression-level coverage and Go is statement-level only.)

[rogpeppe]

@alandonovan I agree in principle, but care will be needed to avoid accidentally mixing incompatible coverage reports. I think it's probably OK if we're using modules and all the modules used to build the binary have the same versions as the ones in the parent, but currently the coverage file doesn't provide any way of verifying this.

The way that Blaze does it sounds like a decent approach - could we change Go's stdlib support to be a bit more like that, or would that founder on backward-compatibility issues?

[alandonovan]

What I'm proposing is essentially that testing's coverage machinery be split into another package on which it depends; the existing API can remain and delegate. The new cover package would expose a CoverReport function; all other things that it exposes would be excluded from the API stability guarantee, just as they are excluded today in the testing package. This would provides a standard way to flush a coverage report, whether or not an application depends on testing. I don't see any obvious compatibility problems other than those of depending on new function from the standard library.

What do you mean by incompatible coverage reports? I would expect the cover-transformed source files to ephemeral, never checked in, so the cover tool and the testing (or cover) packages can always assume they are at the same Go version.

[andybons]

@rogpeppe any thoughts on @alandonovan's question?

[andybons]

Ping @rogpeppe

[gopherbot]

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

[mvdan]

I think this fell under @rogpeppe's radar as he's recently switched jobs and participated in larger proposals, like the error ones. Hopefully he can chime in at some point soon and we can reopen this issue.