Make coverage work with BwoB

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -2527,6 +2527,16 @@ java_library(
     ],
 )
 
+java_library(
+    name = "test/coverage_report",
+    srcs = ["test/CoverageReport.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/vfs",
+        "//src/main/java/com/google/devtools/build/lib/events",
+        "//third_party:guava",
+    ],
+)
+
 java_library(
     name = "test/coverage_report_action_factory",
     srcs = ["test/CoverageReportActionFactory.java"],

src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReport.java

@@ -0,0 +1,19 @@
+package com.google.devtools.build.lib.analysis.test;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.vfs.Path;
+
+/** This event is used to notify about a successfully generated coverage report. */
+public final class CoverageReport implements ExtendedEventHandler.Postable {
+  private final ImmutableList<Path> files;
+
+  public CoverageReport(
+      ImmutableList<Path> files) {
+    this.files = files;
+  }
+
+  public ImmutableList<Path> getFiles() {
+    return files;
+  }
+}

src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD

@@ -25,12 +25,14 @@ 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",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_resolver",
         "//src/main/java/com/google/devtools/build/lib/util",
+        "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/common/options",
         "//third_party:auto_value",

src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java

@@ -14,6 +14,8 @@
 
 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;
 import com.google.common.collect.ImmutableMap;
@@ -30,6 +32,7 @@
 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;
@@ -45,6 +48,7 @@
 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;
@@ -57,6 +61,7 @@
 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;
@@ -146,6 +151,11 @@ 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);

src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java

@@ -26,7 +26,9 @@
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputHelper;
+import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
+import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.ExecException;
@@ -52,12 +54,15 @@
 import com.google.devtools.build.lib.server.FailureDetails.Execution.Code;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
 import com.google.devtools.build.lib.server.FailureDetails.TestAction;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.io.FileOutErr;
+import com.google.devtools.build.lib.vfs.Dirent;
 import com.google.devtools.build.lib.vfs.FileStatus;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus;
 import com.google.devtools.build.lib.view.test.TestStatus.TestCase;
 import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
@@ -67,8 +72,10 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.time.Duration;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
 import javax.annotation.Nullable;
 
@@ -777,14 +784,17 @@ public TestAttemptContinuation execute()
             closeSuppressed(e, fileOutErr);
             throw e;
           }
-          actionExecutionContext
-              .getMetadataHandler()
-              .getMetadata(testAction.getCoverageDirectoryTreeArtifact());
-          ImmutableSet<? extends ActionInput> expandedCoverageDir =
-              actionExecutionContext
-                  .getMetadataHandler()
-                  .getTreeArtifactChildren(
-                      (SpecialArtifact) testAction.getCoverageDirectoryTreeArtifact());
+          Path coverageDir = actionExecutionContext.getPathResolver().convertPath(
+              testAction.getCoverageDirectoryTreeArtifact().getPath());
+          Set<ActionInput> expandedCoverageDir = new HashSet<>();
+          TreeArtifactValue.visitTree(coverageDir, ((parentRelativePath, type) -> {
+            if (type == Dirent.Type.DIRECTORY) {
+              return;
+            }
+            expandedCoverageDir.add(TreeFileArtifact.createTreeOutput(
+                (SpecialArtifact) testAction.getCoverageDirectoryTreeArtifact(),
+                parentRelativePath));
+          }));
           Spawn coveragePostProcessingSpawn =
               createCoveragePostProcessingSpawn(
                   actionExecutionContext,
@@ -827,7 +837,8 @@ public TestAttemptContinuation execute()
 
       Verify.verify(
           !(testAction.isCoverageMode() && testAction.getSplitCoveragePostProcessing())
-              || testAction.getCoverageData().getPath().exists());
+              || actionExecutionContext.getPathResolver()
+              .convertPath(testAction.getCoverageData().getPath()).exists());
       Verify.verifyNotNull(spawnResults);
       Verify.verifyNotNull(testResultDataBuilder);
 

src/main/java/com/google/devtools/build/lib/remote/BUILD

@@ -199,6 +199,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
+        "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_report",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/remote/util",

src/main/java/com/google/devtools/build/lib/remote/ToplevelArtifactsDownloader.java

@@ -35,6 +35,7 @@
 import com.google.devtools.build.lib.analysis.TargetCompleteEvent;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
+import com.google.devtools.build.lib.analysis.test.CoverageReport;
 import com.google.devtools.build.lib.analysis.test.TestAttempt;
 import com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher.Priority;
 import com.google.devtools.build.lib.remote.util.StaticMetadataProvider;
@@ -56,7 +57,8 @@ private enum CommandMode {
     UNKNOWN,
     BUILD,
     TEST,
-    RUN;
+    RUN,
+    COVERAGE;
   }
 
   private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
@@ -83,6 +85,9 @@ public ToplevelArtifactsDownloader(
       case "run":
         this.commandMode = CommandMode.RUN;
         break;
+      case "coverage":
+        this.commandMode = CommandMode.COVERAGE;
+        break;
       default:
         this.commandMode = CommandMode.UNKNOWN;
     }
@@ -108,28 +113,40 @@ public interface PathToMetadataConverter {
   @AllowConcurrentEvents
   public void onTestAttempt(TestAttempt event) {
     for (Pair<String, Path> pair : event.getFiles()) {
-      Path path = checkNotNull(pair.getSecond());
-      // Since the event is fired within action execution, the skyframe doesn't know the outputs of
-      // test actions yet, so we can't get their metadata through skyframe. However, the fileSystem
-      // of the path is an ActionFileSystem, we use it to get the metadata for this file.
-      FileArtifactValue metadata = pathToMetadataConverter.getMetadata(path);
-      if (metadata != null) {
-        ListenableFuture<Void> future =
-            actionInputPrefetcher.downloadFileAsync(path.asFragment(), metadata, Priority.LOW);
-        addCallback(
-            future,
-            new FutureCallback<Void>() {
-              @Override
-              public void onSuccess(Void unused) {}
+      downloadActionOutput(checkNotNull(pair.getSecond()));
+    }
+  }
 
-              @Override
-              public void onFailure(Throwable throwable) {
-                logger.atWarning().withCause(throwable).log(
-                    "Failed to download test output %s.", path);
-              }
-            },
-            directExecutor());
-      }
+  @Subscribe
+  @AllowConcurrentEvents
+  public void onCoverageReport(CoverageReport event) {
+    for (var file : event.getFiles()) {
+      downloadActionOutput(file);
+    }
+  }
+
+  // This should only be used if the event is fired within action execution, in which case, the
+  // skyframe doesn't know the outputs of test actions yet, so we can't get their metadata through
+  // skyframe. However, the fileSystem of the path is an ActionFileSystem, we use it to get the
+  // metadata for this file.
+  private void downloadActionOutput(Path path) {
+    FileArtifactValue metadata = pathToMetadataConverter.getMetadata(path);
+    if (metadata != null) {
+      ListenableFuture<Void> future =
+          actionInputPrefetcher.downloadFileAsync(path.asFragment(), metadata, Priority.LOW);
+      addCallback(
+          future,
+          new FutureCallback<Void>() {
+            @Override
+            public void onSuccess(Void unused) {}
+
+            @Override
+            public void onFailure(Throwable throwable) {
+              logger.atWarning().withCause(throwable).log(
+                  "Failed to download test output %s.", path);
+            }
+          },
+          directExecutor());
     }
   }
 
@@ -168,8 +185,9 @@ private boolean shouldDownloadToplevelOutputs(ConfiguredTargetKey configuredTarg
       case RUN:
         // Always download outputs of toplevel targets in RUN mode
         return true;
+      case COVERAGE:
       case TEST:
-        // Do not download test binary in test mode.
+        // Do not download test binary in test/coverage mode.
         try {
           var configuredTargetValue =
               (ConfiguredTargetValue) memoizingEvaluator.getExistingValue(configuredTargetKey);

src/test/shell/bazel/remote/build_without_the_bytes_test.sh

@@ -1353,4 +1353,85 @@ EOF
   expect_log "bin-message"
 }
 
+function test_java_rbe_coverage_produces_report_with_remote_download_minimal() {
+  mkdir -p java/factorial
+
+  JAVA_TOOLS_ZIP="released"
+  COVERAGE_GENERATOR_DIR="released"
+
+  cd java/factorial
+
+  cat > BUILD <<'EOF'
+java_library(
+    name = "fact",
+    srcs = ["Factorial.java"],
+)
+java_test(
+    name = "fact-test",
+    size = "small",
+    srcs = ["FactorialTest.java"],
+    test_class = "factorial.FactorialTest",
+    deps = [
+        ":fact",
+    ],
+)
+EOF
+
+  cat > Factorial.java <<'EOF'
+package factorial;
+public class Factorial {
+  public static int factorial(int x) {
+    return x <= 0 ? 1 : x * factorial(x-1);
+  }
+}
+EOF
+
+  cat > FactorialTest.java <<'EOF'
+package factorial;
+import static org.junit.Assert.*;
+import org.junit.Test;
+public class FactorialTest {
+  @Test
+  public void testFactorialOfZeroIsOne() throws Exception {
+    assertEquals(Factorial.factorial(3),6);
+  }
+}
+EOF
+  cd ../..
+
+  cat $(rlocation io_bazel/src/test/shell/bazel/testdata/jdk_http_archives) >> WORKSPACE
+
+  bazel coverage \
+    --test_output=all \
+    --experimental_fetch_all_coverage_outputs \
+    --experimental_split_coverage_postprocessing \
+    --remote_download_minimal \
+    --combined_report=lcov \
+    --spawn_strategy=remote \
+    --remote_executor=grpc://localhost:${worker_port} \
+    --instrumentation_filter=//java/factorial \
+    //java/factorial:fact-test &> $TEST_log || fail "Shouldn't fail"
+
+  local expected_result="SF:java/factorial/Factorial.java
+FN:2,factorial/Factorial::<init> ()V
+FN:4,factorial/Factorial::factorial (I)I
+FNDA:0,factorial/Factorial::<init> ()V
+FNDA:1,factorial/Factorial::factorial (I)I
+FNF:2
+FNH:1
+BRDA:4,0,0,1
+BRDA:4,0,1,1
+BRF:2
+BRH:2
+DA:2,0
+DA:4,1
+LH:1
+LF:2
+end_of_record"
+  cat bazel-testlogs/java/factorial/fact-test/coverage.dat > $TEST_log
+  expect_log "$expected_result"
+  cat bazel-out/_coverage/_coverage_report.dat > $TEST_log
+  expect_log "$expected_result"
+}
+
 run_suite "Build without the Bytes tests"