Generate combined coverage artifacts after exclusive tests have run.

The combined coverage report requires tests to be executed in order to
be built. If it is requested as part of the build then those tests will
be executed.

This fixes SkyframeBuilder and SkyframeBuildView to trigger generation
of the combined report (or all coverage artifacts in the case of the
latter) only after all exclusive tests have been run.

Fixes #6005

PiperOrigin-RevId: 550869458
Change-Id: I67d28184e59618a756b0114bf77996e3e9c26ffb

Author: c-mita

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

@@ -403,6 +403,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe:configuration_phase_started_event",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_progress_receiver",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:coverage_report_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:default_syscall_cache",
         "//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness",
         "//src/main/java/com/google/devtools/build/lib/skyframe:execution_finished_event",

src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java

@@ -13,6 +13,9 @@
 // limitations under the License.
 package com.google.devtools.build.lib.buildtool;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.devtools.build.lib.skyframe.CoverageReportValue.COVERAGE_REPORT_KEY;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
@@ -142,6 +145,14 @@ public void buildArtifacts(
         executionProgressReceiver, statusReporter);
     watchdog.start();
 
+    // We need to extract out artifacts for the combined coverage report; these should only be built
+    // after any exclusive tests have been run, otherwise the tests get run as part of the build.
+    ImmutableSet<Artifact> coverageReportArtifacts =
+        artifacts.stream()
+            .filter(artifact -> artifact.getArtifactOwner().equals(COVERAGE_REPORT_KEY))
+            .collect(toImmutableSet());
+    Set<Artifact> artifactsToBuild = Sets.difference(artifacts, coverageReportArtifacts);
+
     targetsToBuild = Sets.difference(targetsToBuild, targetsToSkip);
     parallelTests = Sets.difference(parallelTests, targetsToSkip);
     exclusiveTests = Sets.difference(exclusiveTests, targetsToSkip);
@@ -152,7 +163,7 @@ public void buildArtifacts(
               reporter,
               resourceManager,
               executor,
-              artifacts,
+              artifactsToBuild,
               targetsToBuild,
               aspects,
               parallelTests,
@@ -206,6 +217,26 @@ public void buildArtifacts(
           detailedExitCodes.add(detailedExitCode);
         }
       }
+      // Build coverage report
+      if (!coverageReportArtifacts.isEmpty()) {
+        result =
+            skyframeExecutor.evaluateSkyKeysWithExecution(
+                reporter,
+                executor,
+                Artifact.keys(coverageReportArtifacts),
+                options,
+                actionCacheChecker);
+        detailedExitCode =
+            SkyframeErrorProcessor.processResult(
+                reporter,
+                result,
+                options.getOptions(KeepGoingOption.class).keepGoing,
+                skyframeExecutor.getCyclesReporter(),
+                bugReporter);
+        if (detailedExitCode != null) {
+          detailedExitCodes.add(detailedExitCode);
+        }
+      }
     } finally {
       watchdog.stop();
       skyframeExecutor.setActionExecutionProgressReportingObjects(null, null, null);

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java

@@ -668,27 +668,6 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
           skyframeExecutor.resetBuildDriverFunction();
           skyframeExecutor.setTestTypeResolver(null);
 
-          // Coverage needs to be done after the list of analyzed targets/tests is known.
-          ImmutableSet<Artifact> coverageArtifacts =
-              coverageReportActionsWrapperSupplier.getCoverageArtifacts(
-                  buildResultListener.getAnalyzedTargets(), buildResultListener.getAnalyzedTests());
-          eventBus.post(CoverageArtifactsKnownEvent.create(coverageArtifacts));
-          additionalArtifactsResult =
-              skyframeExecutor.evaluateSkyKeys(
-                  eventHandler, Artifact.keys(coverageArtifacts), keepGoing);
-          eventBus.post(new CoverageActionFinishedEvent());
-          if (additionalArtifactsResult.hasError()) {
-            detailedExitCodes.add(
-                SkyframeErrorProcessor.processErrors(
-                        additionalArtifactsResult,
-                        skyframeExecutor.getCyclesReporter(),
-                        eventHandler,
-                        keepGoing,
-                        eventBus,
-                        bugReporter,
-                        /* includeExecutionPhase= */ true)
-                    .executionDetailedExitCode());
-          }
           // These attributes affect whether conflict checking will be done during the next build.
           if (shouldCheckForConflicts(checkForActionConflicts, newKeys)) {
             largestTopLevelKeySetCheckedForConflicts = newKeys;
@@ -698,8 +677,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
 
         // The exclusive tests whose analysis succeeded i.e. those that can be run.
         ImmutableSet<ConfiguredTarget> exclusiveTestsToRun = getExclusiveTests(evaluationResult);
-        boolean continueWithExclusiveTests =
-            (!evaluationResult.hasError() && !additionalArtifactsResult.hasError()) || keepGoing;
+        boolean continueWithExclusiveTests = !evaluationResult.hasError() || keepGoing;
 
         if (continueWithExclusiveTests && !exclusiveTestsToRun.isEmpty()) {
           skyframeExecutor.getIsBuildingExclusiveArtifacts().set(true);
@@ -729,6 +707,29 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
             }
           }
         }
+        // Coverage report generation should only be requested after all tests have executed.
+        // We could generate baseline coverage artifacts earlier; it is only the timing of the
+        // combined report that matters.
+        ImmutableSet<Artifact> coverageArtifacts =
+            coverageReportActionsWrapperSupplier.getCoverageArtifacts(
+                buildResultListener.getAnalyzedTargets(), buildResultListener.getAnalyzedTests());
+        eventBus.post(CoverageArtifactsKnownEvent.create(coverageArtifacts));
+        additionalArtifactsResult =
+            skyframeExecutor.evaluateSkyKeys(
+                eventHandler, Artifact.keys(coverageArtifacts), keepGoing);
+        eventBus.post(new CoverageActionFinishedEvent());
+        if (additionalArtifactsResult.hasError()) {
+          detailedExitCodes.add(
+              SkyframeErrorProcessor.processErrors(
+                      additionalArtifactsResult,
+                      skyframeExecutor.getCyclesReporter(),
+                      eventHandler,
+                      keepGoing,
+                      eventBus,
+                      bugReporter,
+                      /* includeExecutionPhase= */ true)
+                  .executionDetailedExitCode());
+        }
       } finally {
         // No more action execution beyond this point.
         skyframeExecutor.clearExecutionStatesSkymeld(eventHandler);

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

@@ -1828,6 +1828,23 @@ private EvaluationResult<SkyValue> evaluateSkyKeys(
     return evaluateSkyKeys(eventHandler, skyKeys, false);
   }
 
+  /** Evaluates sky keys that require action execution and returns their evaluation results. */
+  public EvaluationResult<SkyValue> evaluateSkyKeysWithExecution(
+      final Reporter reporter,
+      final Executor executor,
+      final Iterable<? extends SkyKey> skyKeys,
+      final OptionsProvider options,
+      final ActionCacheChecker actionCacheChecker) {
+
+    prepareSkyframeActionExecutorForExecution(reporter, executor, options, actionCacheChecker);
+    try {
+      return evaluateSkyKeys(
+          reporter, skyKeys, options.getOptions(KeepGoingOption.class).keepGoing);
+    } finally {
+      cleanUpAfterSingleEvaluationWithActionExecution(reporter);
+    }
+  }
+
   /**
    * Evaluates the given sky keys, blocks, and returns their evaluation results. Enables/disables
    * "keep going" on evaluation errors as specified.