Add support for exclusive test with Skymeld.
PiperOrigin-RevId: 440939395
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 07d0ea9..917aa98 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -657,12 +657,23 @@
/**
* Check for errors in "chronological" order (acknowledge that loading and analysis are
* interleaved, but sequential on the single target scale).
+ *
+ * <p>For Skymeld: execution errors should take precedence, since those are DetailedExceptions.
*/
@Nullable
public static FailureDetail createFailureDetail(
TargetPatternPhaseValue loadingResult,
@Nullable SkyframeAnalysisResult skyframeAnalysisResult,
@Nullable TopLevelTargetsAndConfigsResult topLevelTargetsAndConfigs) {
+ if (skyframeAnalysisResult instanceof SkyframeAnalysisAndExecutionResult) {
+ SkyframeAnalysisAndExecutionResult skyframeAnalysisAndExecutionResult =
+ (SkyframeAnalysisAndExecutionResult) skyframeAnalysisResult;
+ if (skyframeAnalysisAndExecutionResult.getRepresentativeExecutionExitCode() != null) {
+ return skyframeAnalysisAndExecutionResult
+ .getRepresentativeExecutionExitCode()
+ .getFailureDetail();
+ }
+ }
if (loadingResult.hasError()) {
return FailureDetail.newBuilder()
.setMessage("command succeeded, but there were errors parsing the target pattern")
@@ -692,15 +703,6 @@
.setAnalysis(Analysis.newBuilder().setCode(Analysis.Code.NOT_ALL_TARGETS_ANALYZED))
.build();
}
- if (skyframeAnalysisResult instanceof SkyframeAnalysisAndExecutionResult) {
- SkyframeAnalysisAndExecutionResult skyframeAnalysisAndExecutionResult =
- (SkyframeAnalysisAndExecutionResult) skyframeAnalysisResult;
- if (skyframeAnalysisAndExecutionResult.getRepresentativeExecutionExitCode() != null) {
- return skyframeAnalysisAndExecutionResult
- .getRepresentativeExecutionExitCode()
- .getFailureDetail();
- }
- }
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 4ff1187..f2febef 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -128,6 +128,7 @@
":diff_awareness_manager",
":directory_listing_function",
":directory_listing_state_value",
+ ":exclusive_test_build_driver_value",
":execution_finished_event",
":file_function",
":fileset_entry_function",
@@ -846,6 +847,7 @@
":build_driver_key",
":build_driver_value",
":configured_target_key",
+ ":exclusive_test_build_driver_value",
":incremental_artifact_conflict_finder",
":precomputed_value",
":target_completion_value",
@@ -1242,6 +1244,16 @@
)
java_library(
+ name = "exclusive_test_build_driver_value",
+ srcs = ["ExclusiveTestBuildDriverValue.java"],
+ deps = [
+ ":build_driver_value",
+ "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
+ "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+ ],
+)
+
+java_library(
name = "execution_finished_event",
srcs = ["ExecutionFinishedEvent.java"],
deps = [
@@ -2186,6 +2198,7 @@
":detailed_exceptions",
":sane_analysis_exception",
":sky_functions",
+ ":test_completion_value",
":top_level_conflict_exception",
":transitive_target_key",
"//src/main/java/com/google/devtools/build/lib/actions",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java
index efa6d75..2d90a03 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.devtools.build.lib.skyframe.BuildDriverKey.TestType.EXCLUSIVE;
import static com.google.devtools.build.lib.skyframe.BuildDriverKey.TestType.NOT_TEST;
import static com.google.devtools.build.lib.skyframe.BuildDriverKey.TestType.PARALLEL;
@@ -133,7 +134,17 @@
requestAspectExecution((TopLevelAspectsValue) topLevelSkyValue, env, topLevelArtifactContext);
}
- return env.valuesMissing() ? null : new BuildDriverValue(topLevelSkyValue);
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ if (EXCLUSIVE.equals(buildDriverKey.getTestType())) {
+ Preconditions.checkState(topLevelSkyValue instanceof ConfiguredTargetValue);
+ return new ExclusiveTestBuildDriverValue(
+ topLevelSkyValue, ((ConfiguredTargetValue) topLevelSkyValue).getConfiguredTarget());
+ }
+
+ return new BuildDriverValue(topLevelSkyValue);
}
private void requestConfiguredTargetExecution(
@@ -147,7 +158,7 @@
ImmutableSet.Builder<Artifact> artifactsToBuild = ImmutableSet.builder();
addExtraActionsIfRequested(
configuredTarget.getProvider(ExtraActionArtifactsProvider.class), artifactsToBuild);
- if (buildDriverKey.getTestType() == NOT_TEST) {
+ if (NOT_TEST.equals(buildDriverKey.getTestType())) {
declareDependenciesAndCheckValues(
env,
Iterables.concat(
@@ -171,20 +182,22 @@
state.sentTestAnalysisCompleteEvent = true;
}
- Preconditions.checkState(
- PARALLEL.equals(buildDriverKey.getTestType()),
- "Invalid test type, expect only parallel tests: %s",
- buildDriverKey);
- // Only run non-exclusive tests here. Exclusive tests need to be run sequentially later.
- declareDependenciesAndCheckValues(
- env,
- Iterables.concat(
- artifactsToBuild.build(),
- Collections.singletonList(
- TestCompletionValue.key(
- (ConfiguredTargetKey) actionLookupKey,
- topLevelArtifactContext,
- /*exclusiveTesting=*/ false))));
+ if (PARALLEL.equals(buildDriverKey.getTestType())) {
+ // Only run non-exclusive tests here. Exclusive tests need to be run sequentially later.
+ declareDependenciesAndCheckValues(
+ env,
+ Iterables.concat(
+ artifactsToBuild.build(),
+ Collections.singletonList(
+ TestCompletionValue.key(
+ (ConfiguredTargetKey) actionLookupKey,
+ topLevelArtifactContext,
+ /*exclusiveTesting=*/ false))));
+ return;
+ }
+
+ // Exclusive tests will be run with sequential Skyframe evaluations afterwards.
+ declareDependenciesAndCheckValues(env, artifactsToBuild.build());
}
private void requestAspectExecution(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExclusiveTestBuildDriverValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExclusiveTestBuildDriverValue.java
new file mode 100644
index 0000000..acc0188
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExclusiveTestBuildDriverValue.java
@@ -0,0 +1,33 @@
+// Copyright 2022 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.skyframe.SkyValue;
+
+/** A subclass of BuildDriverValue, meant to represent an exclusive test. */
+public class ExclusiveTestBuildDriverValue extends BuildDriverValue {
+
+ private final ConfiguredTarget exclusiveTestConfiguredTarget;
+
+ ExclusiveTestBuildDriverValue(
+ SkyValue wrappedSkyValue, ConfiguredTarget exclusiveTestConfiguredTarget) {
+ super(wrappedSkyValue);
+ this.exclusiveTestConfiguredTarget = exclusiveTestConfiguredTarget;
+ }
+
+ public ConfiguredTarget getExclusiveTestConfiguredTarget() {
+ return exclusiveTestConfiguredTarget;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index d744b08..100d58c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -87,6 +87,7 @@
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.FailureToRetrieveIntrospectedValueException;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.TopLevelActionConflictReport;
import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.DetailedExitCode.DetailedExitCodeComparator;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.skyframe.ErrorInfo;
@@ -98,6 +99,7 @@
import com.google.devtools.common.options.OptionDefinition;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -642,19 +644,8 @@
}
List<BuildDriverKey> buildDriverCTKeys = new ArrayList<>(/*initialCapacity=*/ ctKeys.size());
- List<Label> exclusiveTestsLabels = new ArrayList<>();
for (ConfiguredTargetKey ctKey : ctKeys) {
- TestType testType =
- determineTestType(
- testsToRun,
- labelTargetMap,
- ctKey.getLabel(),
- topLevelArtifactContext.runTestsExclusively());
- if (TestType.EXCLUSIVE.equals(testType)) {
- exclusiveTestsLabels.add(ctKey.getLabel());
- continue;
- }
buildDriverCTKeys.add(
BuildDriverKey.ofConfiguredTarget(
ctKey,
@@ -667,14 +658,6 @@
topLevelArtifactContext.runTestsExclusively())));
}
- if (!exclusiveTestsLabels.isEmpty()) {
- // TODO(b/223558573) Make exclusive tests work.
- eventHandler.handle(
- Event.warn(
- "--experimental_merged_skyframe_analysis_execution is currently incompatible with"
- + " exclusive tests. The following exclusive tests won't be run: "
- + exclusiveTestsLabels));
- }
List<BuildDriverKey> buildDriverAspectKeys =
topLevelAspectsKeys.stream()
.map(
@@ -699,7 +682,37 @@
skyframeExecutor.resetIncrementalArtifactConflictFindingStates();
}
- if (!evaluationResult.hasError()) {
+ // The exclusive tests whose analysis succeeded i.e. those that can be run.
+ ImmutableSet<ConfiguredTarget> exclusiveTestsToRun = getExclusiveTests(evaluationResult);
+ boolean continueWithExclusiveTests = !evaluationResult.hasError() || keepGoing;
+ List<DetailedExitCode> detailedExitCodes = new ArrayList<>();
+
+ if (continueWithExclusiveTests && !exclusiveTestsToRun.isEmpty()) {
+ // Run exclusive tests sequentially.
+ Iterable<SkyKey> testCompletionKeys =
+ TestCompletionValue.keys(
+ exclusiveTestsToRun, topLevelArtifactContext, /*exclusiveTesting=*/ true);
+ for (SkyKey testCompletionKey : testCompletionKeys) {
+ EvaluationResult<SkyValue> testRunResult =
+ skyframeExecutor.evaluateSkyKeys(
+ eventHandler, ImmutableSet.of(testCompletionKey), keepGoing);
+ if (testRunResult.hasError()) {
+ detailedExitCodes.add(
+ SkyframeErrorProcessor.processErrors(
+ testRunResult,
+ configurationLookupSupplier,
+ skyframeExecutor.getCyclesReporter(),
+ eventHandler,
+ keepGoing,
+ eventBus,
+ bugReporter,
+ /*includeExecutionPhase=*/ true)
+ .executionDetailedExitCode());
+ }
+ }
+ }
+
+ if (!evaluationResult.hasError() && detailedExitCodes.isEmpty()) {
Map<AspectKey, ConfiguredAspect> successfulAspects =
getSuccessfulAspectMap(
topLevelAspectsKeys.size(),
@@ -730,6 +743,7 @@
eventBus,
bugReporter,
/*includeExecutionPhase=*/ true);
+ detailedExitCodes.add(errorProcessingResult.executionDetailedExitCode());
foundActionConflictInLatestCheck = !errorProcessingResult.actionConflicts().isEmpty();
TopLevelActionConflictReport topLevelActionConflictReport =
@@ -763,7 +777,7 @@
evaluationResult.getWalkableGraph(),
ImmutableMap.copyOf(successfulAspects),
/*packageRoots=*/ null,
- errorProcessingResult.executionDetailedExitCode());
+ Collections.max(detailedExitCodes, DetailedExitCodeComparator.INSTANCE));
}
/**
@@ -877,6 +891,18 @@
}
}
+ private static ImmutableSet<ConfiguredTarget> getExclusiveTests(
+ EvaluationResult<BuildDriverValue> evaluationResult) {
+ ImmutableSet.Builder<ConfiguredTarget> exclusiveTests = ImmutableSet.builder();
+ for (BuildDriverValue value : evaluationResult.values()) {
+ if (value instanceof ExclusiveTestBuildDriverValue) {
+ exclusiveTests.add(
+ ((ExclusiveTestBuildDriverValue) value).getExclusiveTestConfiguredTarget());
+ }
+ }
+ return exclusiveTests.build();
+ }
+
private static TestType determineTestType(
ImmutableSet<Label> testsToRun,
ImmutableMap<Label, Target> labelTargetMap,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeErrorProcessor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeErrorProcessor.java
index d1f0c81..29b0df0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeErrorProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeErrorProcessor.java
@@ -53,6 +53,7 @@
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.ArtifactConflictFinder.ConflictException;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.TopLevelAspectsKey;
+import com.google.devtools.build.lib.skyframe.TestCompletionValue.TestCompletionKey;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.DetailedExitCode.DetailedExitCodeComparator;
import com.google.devtools.build.skyframe.CycleInfo;
@@ -336,6 +337,10 @@
if (errorEntry.getKey().argument() instanceof BuildDriverKey) {
return ((BuildDriverKey) errorEntry.getKey().argument()).getActionLookupKey();
}
+ // For exclusive tests.
+ if (errorEntry.getKey().argument() instanceof TestCompletionKey) {
+ return ((TestCompletionKey) errorEntry.getKey().argument()).configuredTargetKey();
+ }
return errorEntry.getKey();
}
