Generate coverage reports in LCOV format

[fmeum]

What type of PR is this?

Feature

What does this PR do? Why is it needed?

With the new --//go/config:cover_format flag set to lcov, go cover coverage reports are converted to lcov. When combined with Bazel's --combined_report=lcov, this allows for Go coverage to show up in cross-language, cross-target coverage reports.

The combined report can be converted to HTML via e.g.

genhtml bazel-out/_coverage/_coverage_report.dat

Which issues(s) does this PR fix?

Fixes #140

Other notes for review

I haven't added anything to the user-facing docs yet, but will do so before this is merged.

[fmeum]

@achew22 Would you be available to review this new feature?

[linzhp]

Only went half way through the PR. I will pick it up later this week.

[fmeum]

@linzhp I pushed one commit that addresses your comments and another one that removes the branch coverage information - it may be misleading and made the PR unnecessarily complicated.

[achew22]

I think you'll need to update https://github.com/bazelbuild/rules_go/blob/master/tests/core/coverage/coverage_test.go to validate both the lcov and the old (gcov?) coverage formats, but then I'd be happy with this PR.

[fmeum]

@achew22 Should I also update docs? If so, which ones?

[achew22]

I just went through the repo and didn't see any documentation that I think needs to be update. There might need to be some changes made to the nogo coverage test as well, but if it passes I'm happy to merge

[fmeum]

@achew22 I made the changes to the tests and also updated the message of the first commit.

[linzhp]

The logic looks good to me. Thanks for working on this.

[linzhp]

We are getting close

[linzhp]

LGTM. @achew22 feel free to merge if you don't have more concerns.

[linzhp]

It's strange that lcov cannot be produced in Uber's Go monorepo after applying this onto the master of rules_go:

1. when I ran bazel coverage with or without --@io_bazel_rules_go//go/config:cover_format=lcov, it doesn't print out the location of coverage.dat.
2. when I open the coverage.dat, it's empty
3. when I ran bazel coverage --combined_report=lcov, the resulting bazel-out/_coverage/_coverage_report.dat is empty too
4. bazel coverage --@io_bazel_rules_go//go/config:cover_format=go_cover works

[fmeum]

@linzhp Did you try this with bazel test //...? Does it work for a smaller subset of tests?

[linzhp]

I only try it in one single test (running bazel test //... in Uber's Go monorepo would blow up my laptop).

[fmeum]

Depending on the source structure, the instrumentation filter may be off. Do you get reports with --instrument_test_targets --instrumentation_filter=^//?

[linzhp]

The instrumentation filter is on:

bazel coverage --@io_bazel_rules_go//go/config:cover_format=lcov //some/internal/path:all
INFO: Writing tracer profile to '/var/folders/j3/f9m101qs7n1bxh0v__vkxwgm0000gn/T/bazel_20220420101656_2zytq'
INFO: Invocation ID: 511beacc-ebf1-45cb-97ae-612b4271f93d
INFO: Using default value for --instrumentation_filter: "^//some/internal/path[/:]".
INFO: Override the above default with --instrumentation_filter
INFO: Build options --@io_bazel_rules_go//go/config:cover_format and --instrumentation_filter have changed, discarding analysis cache.
INFO: Analyzed 4 targets (0 packages loaded, 11096 targets configured).
INFO: Found 3 targets and 1 test target...
INFO: Elapsed time: 0.550s, Critical Path: 0.08s
INFO: 11 processes: 10 disk cache hit, 1 internal.
INFO: Build completed successfully, 11 total actions
//some/internal/path:go_default_test (cached) PASSED in 0.0s

Executed 0 out of 1 test: 1 test passes.

I also tried:

bazel coverage --instrument_test_targets --instrumentation_filter=^// --@io_bazel_rules_go//go/config:cover_format=lcov //some/internal/path:all
INFO: Invocation ID: 170ef37d-ea63-4e0b-82e6-178ee691ca2f
INFO: Build options --instrument_test_targets and --instrumentation_filter have changed, discarding analysis cache.
INFO: Analyzed 4 targets (0 packages loaded, 11096 targets configured).
INFO: Found 3 targets and 1 test target...
INFO: Elapsed time: 1.883s, Critical Path: 1.47s
INFO: 11 processes: 2 internal, 9 darwin-sandbox.
INFO: Build completed successfully, 11 total actions
//some/internal/path:go_default_test    PASSED in 0.3s

Executed 1 out of 1 test: 1 test passes.
INFO: Build completed successfully, 11 total actions

[fmeum]

Which Bazel version are you using at Uber? Could you share any bits of the test log that do not originate in your own code? Maybe there is some helpful output there.

[linzhp]

We are using Bazel 5.1.0. The test.log is like:

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //some/internal/path:go_default_test
-----------------------------------------------------------------------------
PASS
coverage: 61.4% of statements

The testlog directory of that test is like:

total 24
-r-xr-xr-x  1 zplin  wheel    0 Apr 20 09:44 baseline_coverage.dat
-r-xr-xr-x  1 zplin  wheel    0 Apr 20 10:39 coverage.dat
-r-xr-xr-x  1 zplin  wheel  228 Apr 20 10:39 test.cache_status
-r-xr-xr-x  1 zplin  wheel  248 Apr 20 10:39 test.log
drwxr-xr-x  2 zplin  wheel   64 Apr 20 10:39 test.outputs
drwxr-xr-x  2 zplin  wheel   64 Apr 20 10:39 test.outputs_manifest
drwxr-xr-x  2 zplin  wheel   64 Apr 20 10:39 test.raw_splitlogs
-rw-r--r--  1 zplin  wheel  165 Apr 20 10:39 test.xml

[fmeum]

@linzhp Could you post the coverage.dat you get from a run with go_cover format? I think it's most likely that the conversion fails to handle some cases and/or that the code that translates import paths to repo-relative source file paths has issues.

[linzhp]

Are you on Bazel Slack?

[linzhp]

I also tried removing this if statement and write to lcov unconditionally, but still got nothing in the coverage.dat.

[fmeum]

A problem is the func TestMain(m *testing.M) { os.Exit(m.Run()) } pattern, as demonstrated by the failing integration test. The coverage profile is written at the end of m.Run(), but os.Exit() does not run deferred functions. I found a way to hack around this problem.

[linzhp]

The hack works in Uber. @achew22 @robfig @blico thoughts on the hack to support os.Exit(m.Run())? Without the hack, lcov will not work for old Go versions that don't call os.Exit automatically in TestMain, but the hack may break in future Go versions, requiring additional maintenance.

[fmeum]

Just a note: The current minimum supported version of Go with rules_go seems to be 1.16 as cover.go doesn't build with Go 1.15. That aligns well with the hack, which works with Go 1.16+ since it requires the paniconexit0 flag.

[linzhp]

TestMain no longer required calling os.Exit since Go 1.15: https://go.dev/doc/go1.15.

[achew22]

WRT the test.paniconexit0 flag and the comment

// 1. It attains parity with "go test", which enables this feature by default.

Curious about this comment. Do you have a link to go with it? I'd like to see what go test is doing.

[fmeum]

WRT the test.paniconexit0 flag and the comment

// 1. It attains parity with "go test", which enables this feature by default.

Curious about this comment. Do you have a link to go with it? I'd like to see what go test is doing.

It got added in golang/go@4f76fe8

[linzhp]

@fmeum Can you review @abhinav 's improvement on this PR?

[abhinav]

Thanks for that, @linzhp. I forgot to press the button posting this:

Per offline discussion with @linzhp, I made some edits to this PR in fmeum#1.
@fmeum and @linzhp, when you have a chance, please have a look.

[fmeum]

Thanks for the great improvements, @abhinav. I somehow thought that the testdeps package couldn't be referenced from a non-main package since it's under internal, but apparently that's just not true.