Announce combined coverage report on the BES

[fmeum]

This allows BES consumers to access this file without guessing its path and existence, as well as when the actual coverage generation action runs locally.

Also removes an obsolete CoverageReport event.

RELNOTES: The combined coverage report produced via --combined_report=lcov is now announced on the BES via the new CoverageReport event.

[fmeum]

@coeuvre Could you review this?

[michaeledgar]

The feature makes perfect sense and I'm excited to see it submitted! I do think we can implement it without adding a new BEP event type, which is always preferred when possible. In particular, the BuildToolLogs event can be used to report files that contain information collected from the entire build.

Contributing a file to this collection is a bit tricky, but the hooks are there to do this. In general you will extend BazelCoverageReportModule to report this information in the BuildToolLogsCollection.

One roadmap, which is preferred but has a few missing details that you'd need to fill in, is:

1. In BazelCoverageReportModule, add a new static inner class, strawman name CoverageReportCollector.
2. Add to CoverageReportCollector a constructor that takes CoverageReportActionsWrapper and stores it in a final field, strawman name coverageActionsWrapper.
3. Add to CoverageReportCollector a @Subscribe method that takes BuildCompleteEvent that adds the Paths to the BuildToolLogsCollection:
4. Create the CoverageReportCollector and register it with the EventBus inside the CoverageReportActionFactory#createCoverageReportActionsWrapper() definition, after creating the CoverageReportActionsWrapper but before returning it.

    @Subscribe
    public void buildComplete(BuildCompleteEvent event) {
      var paths = extractCoveragePathsTODO(coverageActionsWrapper);
      for (var path : paths) {
        event
            .getResult()
            .getBuildToolLogCollection()
            .addLocalFile("coverage_report.lcov", path); // reusing the name "coverage_report.lcov" doesn't matter.
      }
    }

    private static Iterable<Path> extractCoveragePathsTODO(CoverageReportActionsWrapper coverageActionsWrapper) {
      return null; // TODO: Fill me in.
    }

Another roadmap is:

1. Keep the CoverageReport which does not implement the BEP methods.
2. In BazelCoverageReportModule, add a new static inner class, strawman name CoverageReportCollector.
3. Add to CoverageReportCollector a @Subscribe method that takes CoverageReport and stores the collection of Path objects in a mutable field. This will need to be synchronized and the mutable ImmutableList<Path> field it sets should be @GuardedBy("this")
4. Add to CoverageReportCollector a @Subscribe method, also synchronized, that takes BuildCompleteEvent and adds the Paths to the BuildToolLogsCollection:
5. Create the CoverageReportCollector and register it with the EventBus inside the CoverageReportActionFactory#createCoverageReportActionsWrapper() definition, at/around line 99.

    @Subscribe
    public synchronized void buildComplete(BuildCompleteEvent event) {
      for (var path : coverageReport.getPaths()) {
        event
            .getResult()
            .getBuildToolLogCollection()
            .addLocalFile("coverage_report.lcov", path); // reusing the name "coverage_report.lcov" doesn't matter.
      }
    }

[brentleyjones]

Could code coverage collection be done before the absolute end of the build, and thus would be delayed by adding it to BuildToolLogs? Related: #21475

[fmeum]

@michaeledgar I implemented your first suggestion and could also look into #21475, but any insights from your side would be highly appreciated.

[fmeum]

@michaeledgar Gentle ping, would be great to get this into 7.2.0

[michaeledgar]

@brentleyjones - So for #21745, please correct me if I'm wrong! But it seems to be dealing with the code coverage artifacts produced by individual targets. If accurate, those should definitely not be delayed until the end of the build (skymeld or no skymeld) and the fix is definitely to make sure those events are posted promptly!

For this issue: My understanding after the first review is that it aims to convey a coverage report that is only available after the execution phase is complete. (In the first version, the event was posted after buildArtifacts() is complete). If the execution phase is done, there should be ~no time between "all targets are done building" and "posting BuildToolLogs".

@meum - Commit 680facf looks great to me!

It may not surprise you to learn that this functionality already exists inside Blaze, with a few extra hacks that prevent its direct open-sourcing. But we can someday mash those hacks to fit the open-source contribution you've made, and will have extra motivation to do so now!

[Wyverald]

@meum - Commit 680facf looks great to me!

hey Michael, do you want to approve the PR? Or do you think more work is required here?

[fmeum]

@bazel-io fork 7.2.0