Announce combined coverage report on the BES

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. Closes bazelbuild#22171. PiperOrigin-RevId: 632641499 Change-Id: I0c323a371ec2fdd085bea23a772a85b72c52093f
iancha1992 · May 11, 2024 · bb1fb53 · bb1fb53
1 parent 82d6cf2
commit bb1fb53
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 64 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -2696,16 +2696,6 @@ java_library(
     ],
 )
 
-java_library(
-    name = "test/coverage_report",
-    srcs = ["test/CoverageReport.java"],
-    deps = [
-        "//src/main/java/com/google/devtools/build/lib/events",
-        "//src/main/java/com/google/devtools/build/lib/vfs",
-        "//third_party:guava",
-    ],
-)
-
 java_library(
     name = "test/coverage_configuration",
     srcs = ["test/CoverageConfiguration.java"],

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReport.java b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReport.java
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.analysis.test;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.ActionConflictException;
@@ -71,6 +72,10 @@ public ImmutableList<ActionAnalysisMetadata> getActions() {
     public Collection<Artifact> getCoverageOutputs() {
       return coverageReportAction.getOutputs();
     }
+
+    public Artifact getCoverageReportArtifact() {
+      return Iterables.getOnlyElement(coverageReportAction.getOutputs());
+    }
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD
@@ -27,7 +27,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
         "//src/main/java/com/google/devtools/build/lib/analysis:incompatible_platform_provider",
-        "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_report",
         "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_report_action_factory",
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java
@@ -17,13 +17,16 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.eventbus.EventBus;
+import com.google.common.eventbus.Subscribe;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionLookupKey;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactFactory;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory;
+import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory.CoverageReportActionsWrapper;
+import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.runtime.BlazeModule;
@@ -97,18 +100,21 @@ public CoverageReportActionsWrapper createCoverageReportActionsWrapper(
         }
         Preconditions.checkArgument(options.combinedReport == ReportType.LCOV);
         CoverageReportActionBuilder builder = new CoverageReportActionBuilder();
-        return builder.createCoverageActionsWrapper(
-            eventHandler,
-            directories,
-            targetsToTest,
-            baselineCoverageArtifacts,
-            artifactFactory,
-            actionKeyContext,
-            actionLookupKey,
-            workspaceName,
-            this::getArgs,
-            this::getLocationMessage,
-            /*htmlReport=*/ false);
+        CoverageReportActionsWrapper wrapper =
+            builder.createCoverageActionsWrapper(
+                eventHandler,
+                directories,
+                targetsToTest,
+                baselineCoverageArtifacts,
+                artifactFactory,
+                actionKeyContext,
+                actionLookupKey,
+                workspaceName,
+                this::getArgs,
+                this::getLocationMessage,
+                /* htmlReport= */ false);
+        eventBus.register(new CoverageReportCollector(wrapper));
+        return wrapper;
       }
 
       private ImmutableList<String> getArgs(CoverageArgs args) {
@@ -130,4 +136,14 @@ private String getLocationMessage(CoverageArgs args) {
       }
     };
   }
+
+  private record CoverageReportCollector(CoverageReportActionsWrapper wrapper) {
+    @Subscribe
+    public void buildComplete(BuildCompleteEvent event) {
+      event
+          .getResult()
+          .getBuildToolLogCollection()
+          .addLocalFile("coverage_report.lcov", wrapper.getCoverageReportArtifact().getPath());
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java
@@ -14,7 +14,6 @@
 
 package com.google.devtools.build.lib.bazel.coverage;
 
-import static com.google.common.collect.ImmutableList.toImmutableList;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
@@ -31,7 +30,6 @@
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
 import com.google.devtools.build.lib.actions.ArtifactFactory;
 import com.google.devtools.build.lib.actions.ArtifactOwner;
-import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.BaseSpawn;
 import com.google.devtools.build.lib.actions.ExecException;
@@ -47,7 +45,6 @@
 import com.google.devtools.build.lib.analysis.RunfilesSupport;
 import com.google.devtools.build.lib.analysis.actions.Compression;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
-import com.google.devtools.build.lib.analysis.test.CoverageReport;
 import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory.CoverageReportActionsWrapper;
 import com.google.devtools.build.lib.analysis.test.TestProvider;
 import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams;
@@ -59,7 +56,6 @@
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
 import com.google.devtools.build.lib.util.Fingerprint;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -134,12 +130,6 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)
                 .getContext(SpawnStrategyResolver.class)
                 .exec(spawn, actionExecutionContext);
         actionExecutionContext.getEventHandler().handle(Event.info(locationMessage));
-        ArtifactPathResolver pathResolver = actionExecutionContext.getPathResolver();
-        ImmutableList<Path> files =
-            getOutputs().stream()
-                .map(artifact -> pathResolver.convertPath(artifact.getPath()))
-                .collect(toImmutableList());
-        actionExecutionContext.getEventHandler().post(new CoverageReport(files));
         return ActionResult.create(spawnResults);
       } catch (ExecException e) {
         throw ActionExecutionException.fromExecException(e, this);

diff --git a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh
@@ -1933,6 +1933,7 @@ EOF
     --spawn_strategy=remote \
     --remote_executor=grpc://localhost:${worker_port} \
     --instrumentation_filter=//java/factorial \
+    --build_event_text_file=bes.txt \
     //java/factorial:fact-test &> $TEST_log || fail "Shouldn't fail"
 
   # Test binary shouldn't be downloaded
@@ -1958,6 +1959,10 @@ end_of_record"
   expect_log "$expected_result"
   cat bazel-out/_coverage/_coverage_report.dat > $TEST_log
   expect_log "$expected_result"
+
+  cat bes.txt | tr '\n' ' ' > $TEST_log
+  report_sha=$(sha256sum bazel-out/_coverage/_coverage_report.dat | cut -d ' ' -f 1)
+  expect_log "log {     name: \"coverage_report.lcov\"     uri: \"bytestream://[^\"]*/${report_sha}/[^\"]*\""
 }
 
 function test_remote_cache_eviction_retries() {