Post `TestAttempt`/`TestResult` events when a test can be built but test-exec-config-inputs fail to build.
Usually, when a test target can be built, the `TestRunnerAction` will be
buildable, but this is not always the case. If the special `exec`-config inputs
fail to build, the test action may fail to build, which will only happen in the
`test` command. The build tool (and BEP) assume that if a test-type target is
buildable, and the command is `test`, then the corresponding
`TestAttempt`/`TestResult` events will eventually be posted.
This change ensures those events are posted with minimal, correct information indicating the test `FAILED_TO_BUILD`.
RELNOTES: BEP will include correct \`TestResult\` and \`TargetSummary\` events when special test inputs like \`$test_runtime\` fail to build.
PiperOrigin-RevId: 660904505
Change-Id: Ie44d558be5393ee910e6c4171ac295af2f34b182
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index af2020d..87f8d5f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -198,20 +198,20 @@
return ((DerivedArtifact) artifact).getGeneratingActionKey();
}
- public static Iterable<SkyKey> keys(Iterable<Artifact> artifacts) {
- return artifacts instanceof Collection<Artifact> collection
+ public static <T extends Artifact> Iterable<SkyKey> keys(Iterable<T> artifacts) {
+ return artifacts instanceof Collection<T> collection
? keys(collection)
: Iterables.transform(artifacts, Artifact::key);
}
- public static Collection<SkyKey> keys(Collection<Artifact> artifacts) {
- return artifacts instanceof List<Artifact> list
+ public static <T extends Artifact> Collection<SkyKey> keys(Collection<T> artifacts) {
+ return artifacts instanceof List<T> list
? keys(list)
// Use Collections2 instead of Iterables#transform to ensure O(1) size().
: Collections2.transform(artifacts, Artifact::key);
}
- public static List<SkyKey> keys(List<Artifact> artifacts) {
+ public static <T extends Artifact> List<SkyKey> keys(List<T> artifacts) {
return Lists.transform(artifacts, Artifact::key);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestAttempt.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestAttempt.java
index 9153d3e..4a7aacb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestAttempt.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestAttempt.java
@@ -110,6 +110,10 @@
lastAttempt);
}
+ /**
+ * Creates a test attempt result from cached test data, providing a result while indicating to
+ * consumers that the test did not actually execute.
+ */
public static TestAttempt fromCachedTestResult(
TestRunnerAction testAction,
TestResultData attemptData,
@@ -131,6 +135,29 @@
lastAttempt);
}
+ /**
+ * Creates a test result for rare cases where the test itself was built, but the {@link
+ * TestRunnerAction} could not be started by a test strategy.
+ *
+ * <p>This overload should be very rarely used, and in particular must not be used by an
+ * implementation of a {@link TestStrategy}.
+ */
+ public static TestAttempt forUnstartableTestResult(
+ TestRunnerAction testAction, TestResultData attemptData) {
+ return new TestAttempt(
+ false,
+ testAction,
+ /* executionInfo= */ BuildEventStreamProtos.TestResult.ExecutionInfo.getDefaultInstance(),
+ /* attempt= */ 1,
+ attemptData.getStatus(),
+ attemptData.getStatusDetails(),
+ attemptData.getStartTimeMillisEpoch(),
+ attemptData.getRunDurationMillis(),
+ /* files= */ ImmutableList.of(),
+ attemptData.getWarningList(),
+ /* lastAttempt= */ true);
+ }
+
@VisibleForTesting
public Artifact getTestStatusArtifact() {
return testAction.getCacheStatusArtifact();
@@ -205,7 +232,10 @@
// TODO(b/199940216): Can we populate metadata for these files?
localFiles.add(
new LocalFile(
- file.getSecond(), localFileType, /*artifact=*/ null, /*artifactMetadata=*/ null));
+ file.getSecond(),
+ localFileType,
+ /* artifact= */ null,
+ /* artifactMetadata= */ null));
}
}
return localFiles.build();
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 8c9a159..7bf8943 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -356,6 +356,7 @@
"//src/main/java/com/google/devtools/build/lib/util:TestType",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+ "//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util:heap_offset_helper",
"//src/main/java/com/google/devtools/build/lib/util:resource_usage",
"//src/main/java/com/google/devtools/build/lib/util:string",
@@ -370,6 +371,7 @@
"//src/main/java/net/starlark/java/syntax",
"//src/main/protobuf:failure_details_java_proto",
"//src/main/protobuf:memory_pressure_java_proto",
+ "//src/main/protobuf:test_status_java_proto",
"//third_party:auto_value",
"//third_party:caffeine",
"//third_party:flogger",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
index 8b482fa..c31846a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
@@ -13,17 +13,28 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.ActionExecutionException;
+import com.google.devtools.build.lib.actions.ActionLookupData;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
+import com.google.devtools.build.lib.analysis.test.TestAttempt;
import com.google.devtools.build.lib.analysis.test.TestProvider;
+import com.google.devtools.build.lib.analysis.test.TestResult;
+import com.google.devtools.build.lib.analysis.test.TestRunnerAction;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.skyframe.GraphTraversingHelper;
+import com.google.devtools.build.lib.server.FailureDetails.Execution.Code;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
+import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus;
+import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
+import java.util.List;
import javax.annotation.Nullable;
/**
@@ -39,7 +50,7 @@
(TestCompletionValue.TestCompletionKey) skyKey.argument();
ConfiguredTargetKey ctKey = key.configuredTargetKey();
TopLevelArtifactContext ctx = key.topLevelArtifactContext();
- if (env.getValue(TargetCompletionValue.key(ctKey, ctx, /*willTest=*/ true)) == null) {
+ if (env.getValue(TargetCompletionValue.key(ctKey, ctx, /* willTest= */ true)) == null) {
return null;
}
@@ -58,13 +69,27 @@
}
}
} else {
- if (GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissingMaybeWithExceptions(
- env,
- Iterables.transform(
- TestProvider.getTestStatusArtifacts(ct),
- Artifact.DerivedArtifact::getGeneratingActionKey))) {
+ List<SkyKey> skyKeys = Artifact.keys(TestProvider.getTestStatusArtifacts(ct));
+ SkyframeLookupResult result = env.getValuesAndExceptions(skyKeys);
+ if (env.valuesMissing()) {
return null;
}
+ for (SkyKey actionKey : skyKeys) {
+ try {
+ if (result.getOrThrow(actionKey, ActionExecutionException.class) == null) {
+ return null;
+ }
+ } catch (ActionExecutionException e) {
+ DetailedExitCode detailedExitCode = e.getDetailedExitCode();
+ if (detailedExitCode.getExitCode().equals(ExitCode.BUILD_FAILURE)
+ && ctValue instanceof ActionLookupValue actionLookupValue) {
+ postTestResultEventsForUnbuildableTestInputs(
+ env, (ActionLookupData) actionKey, actionLookupValue, detailedExitCode);
+ } else {
+ return null;
+ }
+ }
+ }
}
return TestCompletionValue.TEST_COMPLETION_MARKER;
}
@@ -73,4 +98,42 @@
public String extractTag(SkyKey skyKey) {
return Label.print(((ConfiguredTargetKey) skyKey.argument()).getLabel());
}
+
+ /**
+ * Posts events for test actions that could not run because one or more exec-configuration inputs
+ * common to all tests failed to build.
+ *
+ * <p>When we run this SkyFunction we will have already built the test executable and its inputs,
+ * but we might fail to build the exec-configuration attributes providing inputs to the {@link
+ * TestRunnerAction} such as {@code $test_runtime}, {@code $test_wrapper}, {@code
+ * test_setup_script} and others (see {@link
+ * com.google.devtools.build.lib.analysis.BaseRuleClasses.TestBaseRule#build(com.google.devtools.build.lib.packages.RuleClass.Builder,
+ * com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment)} where all Test-type rules
+ * have additional attributes added).
+ *
+ * <p>When these exec-configuration inputs cannot be built, we do not get to use any {@code
+ * TestStrategy} that is responsible for posting {@link TestAttempt} and {@link TestResult}
+ * events. We need to handle this case here and post minimal events indicating the test {@link
+ * BlazeTestStatus.FAILED_TO_BUILD FAILED_TO_BUILD}.
+ */
+ private static void postTestResultEventsForUnbuildableTestInputs(
+ Environment env,
+ ActionLookupData actionKey,
+ ActionLookupValue actionLookupValue,
+ DetailedExitCode detailedExitCode) {
+ BlazeTestStatus status = BlazeTestStatus.FAILED_TO_BUILD;
+ if (detailedExitCode
+ .getFailureDetail()
+ .getExecution()
+ .getCode()
+ .equals(Code.ACTION_NOT_UP_TO_DATE)) {
+ status = BlazeTestStatus.NO_STATUS;
+ }
+ TestRunnerAction testRunnerAction =
+ (TestRunnerAction) actionLookupValue.getAction(actionKey.getActionIndex());
+ TestResultData testData = TestResultData.newBuilder().setStatus(status).build();
+ env.getListener().post(TestAttempt.forUnstartableTestResult(testRunnerAction, testData));
+ env.getListener()
+ .post(new TestResult(testRunnerAction, testData, /* cached= */ false, detailedExitCode));
+ }
}
