Automated rollback of commit 64ac2a81f73ba7b085d0de65f12230814f4151c8.
*** Reason for rollback ***
See b/219621210 and b/186996003#comment17
*** Original change description ***
`TestRunnerAction` now produces a `TestResult` when test lifecycle fails at runtime.
Test execution entails running the target's test binary, and even if the binary
has been successfully built, the build tool may encounter failures when spawning
the test executable process. Previously this scenario did not produce a
TestResult or TestSummary BEP event; this is now fixed. Similarly fixed is the
scenario where execution fails after spawning the test process successfully.
The exception-handling l...
PiperOrigin-RevId: 428689515
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
index 2d3af95..a8c7674 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.SpawnResult.Status;
-import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
@@ -69,15 +68,6 @@
@Nullable
ListenableFuture<Void> getTestCancelFuture(ActionOwner owner, int shardNum);
- /**
- * Post the final test result when a test execution is incomplete, perhaps due to environmental
- * failures.
- */
- void finalizeIncompleteTest(
- TestRunnerAction action,
- ActionExecutionContext actionExecutionContext,
- List<FailedAttemptResult> failedAttempts);
-
/** An individual test attempt result. */
interface TestAttemptResult {
/** Test attempt result classification, splitting failures into permanent vs retriable. */
@@ -100,12 +90,6 @@
/** Returns a list of spawn results for this test attempt. */
ImmutableList<SpawnResult> spawnResults();
- /** Returns the TestResultData for the test. */
- TestResultData.Builder testResultDataBuilder();
-
- /** Return the ExecutionInfo data for posting to Build Event Protocol. */
- BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo();
-
/**
* Returns a description of the system failure associated with the primary spawn result, if any.
*/
@@ -208,12 +192,6 @@
FailedAttemptResult finalizeFailedTestAttempt(TestAttemptResult testAttemptResult, int attempt)
throws IOException;
- /**
- * Post the final test result when a test execution is incomplete, perhaps due to environmental
- * failures.
- */
- void finalizeIncompleteTest(TestRunnerAction action, List<FailedAttemptResult> failedAttempts);
-
/** Post the final test result based on the last attempt and the list of failed attempts. */
void finalizeTest(
TestAttemptResult lastTestAttemptResult, List<FailedAttemptResult> failedAttempts)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
index 24333f3..c68157f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -935,36 +935,33 @@
public ActionContinuationOrResult beginExecution(
ActionExecutionContext actionExecutionContext, TestActionContext testActionContext)
throws InterruptedException, ActionExecutionException {
- TestRunnerSpawn testRunnerSpawn;
try {
- testRunnerSpawn = testActionContext.createTestRunnerSpawn(this, actionExecutionContext);
- } catch (ExecException e) {
- testActionContext.finalizeIncompleteTest(this, actionExecutionContext, ImmutableList.of());
- throw ActionExecutionException.fromExecException(e, this);
- }
- ListenableFuture<Void> cancelFuture = null;
- TestAttemptContinuation testAttemptContinuation;
- if (cancelConcurrentTestsOnSuccess) {
- cancelFuture = testActionContext.getTestCancelFuture(getOwner(), shardNum);
- }
- try {
- testAttemptContinuation = beginIfNotCancelled(testRunnerSpawn, cancelFuture);
+ TestRunnerSpawn testRunnerSpawn =
+ testActionContext.createTestRunnerSpawn(this, actionExecutionContext);
+ ListenableFuture<Void> cancelFuture = null;
+ if (cancelConcurrentTestsOnSuccess) {
+ cancelFuture = testActionContext.getTestCancelFuture(getOwner(), shardNum);
+ }
+ TestAttemptContinuation testAttemptContinuation =
+ beginIfNotCancelled(testRunnerSpawn, cancelFuture);
if (testAttemptContinuation == null) {
testRunnerSpawn.finalizeCancelledTest(ImmutableList.of());
// We need to create the mandatory output files even if we're not going to run anything.
createEmptyOutputs(actionExecutionContext);
return ActionContinuationOrResult.of(ActionResult.create(ImmutableList.of()));
}
+ return new RunAttemptsContinuation(
+ this,
+ testRunnerSpawn,
+ testAttemptContinuation,
+ testActionContext.isTestKeepGoing(),
+ cancelFuture);
+ } catch (ExecException e) {
+ throw ActionExecutionException.fromExecException(e, this);
} catch (IOException e) {
throw ActionExecutionException.fromExecException(
new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION), this);
}
- return new RunAttemptsContinuation(
- this,
- testRunnerSpawn,
- testAttemptContinuation,
- testActionContext.isTestKeepGoing(),
- cancelFuture);
}
@Nullable
@@ -1228,8 +1225,6 @@
@Override
public ActionContinuationOrResult execute()
throws ActionExecutionException, InterruptedException {
- ActionContinuationOrResult nextAttemptOrResult;
- TestAttemptResult result;
try {
TestAttemptContinuation nextContinuation;
try {
@@ -1256,93 +1251,80 @@
cancelFuture);
}
- result = nextContinuation.get();
+ TestAttemptResult result = nextContinuation.get();
int actualMaxAttempts =
failedAttempts.isEmpty() ? testRunnerSpawn.getMaxAttempts(result) : maxAttempts;
Preconditions.checkState(actualMaxAttempts != 0);
- TestRunnerSpawnAndMaxAttempts nextRunnerAndAttempts = null;
- if (result.result() != TestAttemptResult.Result.PASSED) {
- nextRunnerAndAttempts =
- computeNextRunnerAndMaxAttempts(
- result.result(), testRunnerSpawn, failedAttempts.size() + 1, actualMaxAttempts);
- }
- // If we made it this far, we finalize and post the test result in the #process() method.
- // Otherwise we finalize in the exception handlers.
- nextAttemptOrResult = process(result, nextRunnerAndAttempts);
+ return process(result, actualMaxAttempts);
} catch (ExecException e) {
- testRunnerSpawn.finalizeIncompleteTest(testRunnerAction, failedAttempts);
- throw ActionExecutionException.fromExecException(e, testRunnerAction);
+ throw ActionExecutionException.fromExecException(e, this.testRunnerAction);
} catch (IOException e) {
throw ActionExecutionException.fromExecException(
new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION),
this.testRunnerAction);
}
- // At this point, we know that the test attempt was finalized.
- TestAttemptResult.Result testResult = result.result();
- if (!keepGoing && testResult != TestAttemptResult.Result.PASSED) {
- DetailedExitCode systemFailure = result.primarySystemFailure();
- if (systemFailure != null) {
- throw ActionExecutionException.fromExecException(
- new TestExecException(
- "Test failed (system error), aborting: "
- + systemFailure.getFailureDetail().getMessage(),
- systemFailure.getFailureDetail()),
- testRunnerAction);
- }
- String errorMessage = "Test failed, aborting";
- throw ActionExecutionException.fromExecException(
- new TestExecException(
- errorMessage,
- FailureDetail.newBuilder()
- .setTestAction(
- TestAction.newBuilder().setCode(TestAction.Code.NO_KEEP_GOING_TEST_FAILURE))
- .setMessage(errorMessage)
- .build()),
- testRunnerAction);
- }
- return nextAttemptOrResult;
}
- private ActionContinuationOrResult process(
- TestAttemptResult result, @Nullable TestRunnerSpawnAndMaxAttempts nextRunnerAndAttempts)
- throws IOException, InterruptedException {
+ private ActionContinuationOrResult process(TestAttemptResult result, int actualMaxAttempts)
+ throws ExecException, IOException, InterruptedException {
spawnResults.addAll(result.spawnResults());
TestAttemptResult.Result testResult = result.result();
if (testResult == TestAttemptResult.Result.PASSED) {
if (cancelFuture != null) {
cancelFuture.cancel(true);
}
- testRunnerSpawn.finalizeTest(result, failedAttempts);
- } else if (nextRunnerAndAttempts == null) {
- testRunnerSpawn.finalizeTest(result, failedAttempts);
} else {
- failedAttempts.add(
- testRunnerSpawn.finalizeFailedTestAttempt(result, failedAttempts.size() + 1));
+ TestRunnerSpawnAndMaxAttempts nextRunnerAndAttempts =
+ computeNextRunnerAndMaxAttempts(
+ testResult, testRunnerSpawn, failedAttempts.size() + 1, actualMaxAttempts);
+ if (nextRunnerAndAttempts != null) {
+ failedAttempts.add(
+ testRunnerSpawn.finalizeFailedTestAttempt(result, failedAttempts.size() + 1));
- TestAttemptContinuation nextContinuation =
- beginIfNotCancelled(nextRunnerAndAttempts.getSpawn(), cancelFuture);
- if (nextContinuation == null) {
- testRunnerSpawn.finalizeCancelledTest(failedAttempts);
- // We need to create the mandatory output files even if we're not going to run anything.
- testRunnerAction.createEmptyOutputs(testRunnerSpawn.getActionExecutionContext());
- return ActionContinuationOrResult.of(ActionResult.create(spawnResults));
+ TestAttemptContinuation nextContinuation =
+ beginIfNotCancelled(nextRunnerAndAttempts.getSpawn(), cancelFuture);
+ if (nextContinuation == null) {
+ testRunnerSpawn.finalizeCancelledTest(failedAttempts);
+ // We need to create the mandatory output files even if we're not going to run anything.
+ testRunnerAction.createEmptyOutputs(testRunnerSpawn.getActionExecutionContext());
+ return ActionContinuationOrResult.of(ActionResult.create(spawnResults));
+ }
+
+ // Change the phase here because we are executing a rerun of the failed attempt.
+ this.testRunnerSpawn
+ .getActionExecutionContext()
+ .getEventHandler()
+ .post(new SpawnExecutedEvent.ChangePhase(this.testRunnerAction));
+
+ return new RunAttemptsContinuation(
+ testRunnerAction,
+ nextRunnerAndAttempts.getSpawn(),
+ nextContinuation,
+ keepGoing,
+ nextRunnerAndAttempts.getMaxAttempts(),
+ spawnResults,
+ failedAttempts,
+ cancelFuture);
}
+ }
+ testRunnerSpawn.finalizeTest(result, failedAttempts);
- // Change the phase here because we are executing a rerun of the failed attempt.
- this.testRunnerSpawn
- .getActionExecutionContext()
- .getEventHandler()
- .post(new SpawnExecutedEvent.ChangePhase(this.testRunnerAction));
-
- return new RunAttemptsContinuation(
- testRunnerAction,
- nextRunnerAndAttempts.getSpawn(),
- nextContinuation,
- keepGoing,
- nextRunnerAndAttempts.getMaxAttempts(),
- spawnResults,
- failedAttempts,
- cancelFuture);
+ if (!keepGoing && testResult != TestAttemptResult.Result.PASSED) {
+ DetailedExitCode systemFailure = result.primarySystemFailure();
+ if (systemFailure != null) {
+ throw new TestExecException(
+ "Test failed (system error), aborting: "
+ + systemFailure.getFailureDetail().getMessage(),
+ systemFailure.getFailureDetail());
+ }
+ String errorMessage = "Test failed, aborting";
+ throw new TestExecException(
+ errorMessage,
+ FailureDetail.newBuilder()
+ .setTestAction(
+ TestAction.newBuilder().setCode(TestAction.Code.NO_KEEP_GOING_TEST_FAILURE))
+ .setMessage(errorMessage)
+ .build());
}
return ActionContinuationOrResult.of(ActionResult.create(spawnResults));
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java
index c70415c..01da1f7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java
@@ -31,7 +31,6 @@
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.config.PerLabelOptions;
import com.google.devtools.build.lib.analysis.test.TestRunnerAction.ResolvedPaths;
-import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventKind;
@@ -68,16 +67,6 @@
private final ConcurrentHashMap<ShardKey, ListenableFuture<Void>> futures =
new ConcurrentHashMap<>();
- /** Base implementation of TestRunnerSpawn with common functionality across implementations. */
- public abstract class AbstractTestRunnerSpawn implements TestRunnerSpawn {
-
- @Override
- public final void finalizeIncompleteTest(
- TestRunnerAction action, List<FailedAttemptResult> failedAttempts) {
- TestStrategy.this.finalizeIncompleteTest(action, getActionExecutionContext(), failedAttempts);
- }
- }
-
/**
* Ensures that all directories used to run test are in the correct state and their content will
* not result in stale files.
@@ -229,6 +218,7 @@
* <p>For rules with "flaky = 1" attribute, this method will return 3 unless --flaky_test_attempts
* option is given and specifies another value.
*/
+ @VisibleForTesting /* protected */
public int getTestAttempts(TestRunnerAction action) {
return action.getTestProperties().isFlaky()
? getTestAttemptsForFlakyTest(action)
@@ -272,54 +262,12 @@
.get(testAction.getTestProperties().getTimeout());
}
- /** Converts the test results to a TestAttemptResult. */
- public abstract TestActionContext.TestAttemptResult makeIncompleteTestResult();
-
- /**
- * Post the final test result when a test execution is incomplete, perhaps due to environmental
- * failures.
+ /*
+ * Finalize test run: persist the result, and post on the event bus.
*/
- @Override
- public void finalizeIncompleteTest(
- TestRunnerAction action,
- ActionExecutionContext actionExecutionContext,
- List<FailedAttemptResult> failedAttempts) {
- TestAttemptResult testResult = makeIncompleteTestResult();
- TestResultData.Builder data = testResult.testResultDataBuilder();
- BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo = testResult.executionInfo();
- int attemptId = failedAttempts.size() + 1;
-
- TestAttempt testAttempt =
- TestAttempt.forExecutedTestResult(
- action,
- data.build(),
- attemptId,
- /*files=*/ ImmutableList.of(),
- executionInfo,
- /*lastAttempt=*/ true);
- TestResult result =
- new TestResult(action, data.build(), false, testResult.primarySystemFailure());
- actionExecutionContext.getEventHandler().post(testAttempt);
- postTestResultNeverCached(actionExecutionContext, result);
- }
-
- /** Report test run: post on the event bus, and write the result to disk cache. */
- public final void postTestResultCached(
- ActionExecutionContext actionExecutionContext, TestResult result) throws IOException {
+ protected void postTestResult(ActionExecutionContext actionExecutionContext, TestResult result)
+ throws IOException {
result.getTestAction().saveCacheStatus(actionExecutionContext, result.getData());
- postTestResultNeverCached(actionExecutionContext, result);
- }
-
- /**
- * Report a test run to the event bus.
- *
- * <p>Called by {@link #postTestResultCached(ActionExecutionContext, TestResult)}. May be called
- * directly for results unsuitable for caching (eg. environmental failure to run the test
- * executable).
- */
- // Not final to support testing.
- protected void postTestResultNeverCached(
- ActionExecutionContext actionExecutionContext, TestResult result) {
actionExecutionContext.getEventHandler().post(result);
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestResult.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestResult.java
index 0947fbd4..5c757bf 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestResult.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestResult.java
@@ -38,10 +38,8 @@
public abstract ImmutableList<SpawnResult> spawnResults();
/** Returns the TestResultData for the test. */
- @Override
public abstract TestResultData.Builder testResultDataBuilder();
- @Override
public abstract BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo();
/** Returns a builder that can be used to construct a {@link StandaloneTestResult} object. */
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
index 8aa0bff..b1c9342 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
@@ -234,16 +234,7 @@
attemptId, /*isLastAttempt=*/ false, actionExecutionContext, action, result);
}
- @Override
- public TestAttemptResult makeIncompleteTestResult() {
- return StandaloneTestResult.builder()
- .setSpawnResults(ImmutableList.of())
- .setExecutionInfo(ExecutionInfo.getDefaultInstance())
- .setTestResultDataBuilder(TestResultData.newBuilder().setStatus(BlazeTestStatus.INCOMPLETE))
- .build();
- }
-
- public void finalizeTest(
+ private void finalizeTest(
TestRunnerAction action,
ActionExecutionContext actionExecutionContext,
StandaloneTestResult standaloneTestResult,
@@ -270,7 +261,7 @@
TestResultData data = dataBuilder.build();
TestResult result =
new TestResult(action, data, false, standaloneTestResult.primarySystemFailure());
- postTestResultCached(actionExecutionContext, result);
+ postTestResult(actionExecutionContext, result);
}
private StandaloneFailedAttemptResult processTestAttempt(
@@ -648,7 +639,7 @@
}
}
- private final class StandaloneTestRunnerSpawn extends AbstractTestRunnerSpawn {
+ private final class StandaloneTestRunnerSpawn implements TestRunnerSpawn {
private final TestRunnerAction testAction;
private final ActionExecutionContext actionExecutionContext;
private final Spawn spawn;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
index 69d53fd..7c2d963 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
@@ -23,7 +23,6 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
import java.io.IOException;
-import java.util.List;
/**
* Test strategy wrapper called 'exclusive'. It should delegate to a test strategy for local
@@ -64,12 +63,4 @@
// aborts of concurrent actions. It's not clear what, if anything, we should do here.
return null;
}
-
- @Override
- public void finalizeIncompleteTest(
- TestRunnerAction action,
- ActionExecutionContext actionExecutionContext,
- List<FailedAttemptResult> failedAttempts) {
- parent.finalizeIncompleteTest(action, actionExecutionContext, failedAttempts);
- }
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
index 9dc65e8..456eac0 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
@@ -14,19 +14,17 @@
package com.google.devtools.build.lib.exec;
-import static com.google.common.collect.MoreCollectors.onlyElement;
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
+import com.google.common.collect.MoreCollectors;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -35,13 +33,11 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.DiscoveredModulesPruner;
-import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnContinuation;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.SpawnResult.Status;
import com.google.devtools.build.lib.actions.SpawnStrategy;
-import com.google.devtools.build.lib.actions.TestExecException;
import com.google.devtools.build.lib.actions.ThreadStateReceiver;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
@@ -68,7 +64,6 @@
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
-import com.google.devtools.build.lib.server.FailureDetails.TestAction;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.io.FileOutErr;
@@ -100,46 +95,20 @@
FailureDetail.newBuilder()
.setSpawn(FailureDetails.Spawn.newBuilder().setCode(Code.NON_ZERO_EXIT))
.build();
- private static final FailureDetail BAD_FLAGS_FAILURE_DETAIL =
- FailureDetail.newBuilder()
- .setTestAction(TestAction.newBuilder().setCode(TestAction.Code.LOCAL_TEST_PREREQ_UNMET))
- .setMessage("bad flags")
- .build();
private static class TestedStandaloneTestStrategy extends StandaloneTestStrategy {
- private final ExecException createTestRunnerSpawnException;
-
TestResult postedResult = null;
public TestedStandaloneTestStrategy(
ExecutionOptions executionOptions, BinTools binTools, Path tmpDirRoot) {
- this(executionOptions, binTools, tmpDirRoot, null);
- }
-
- public TestedStandaloneTestStrategy(
- ExecutionOptions executionOptions,
- BinTools binTools,
- Path tmpDirRoot,
- ExecException createTestRunnerSpawnException) {
super(executionOptions, binTools, tmpDirRoot);
- this.createTestRunnerSpawnException = createTestRunnerSpawnException;
}
@Override
- protected void postTestResultNeverCached(
+ protected void postTestResult(
ActionExecutionContext actionExecutionContext, TestResult result) {
postedResult = result;
}
-
- @Override
- public TestRunnerSpawn createTestRunnerSpawn(
- TestRunnerAction action, ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException {
- if (createTestRunnerSpawnException != null) {
- throw createTestRunnerSpawnException;
- }
- return super.createTestRunnerSpawn(action, actionExecutionContext);
- }
}
private static ActionContext.ActionContextRegistry toContextRegistry(
@@ -342,12 +311,14 @@
assertThat(result.getData().getRunDurationMillis()).isEqualTo(10);
assertThat(result.getData().getTestTimesList()).containsExactly(10L);
TestAttempt attempt =
- storedEvents.getPosts().stream()
+ storedEvents
+ .getPosts()
+ .stream()
.filter(TestAttempt.class::isInstance)
.map(TestAttempt.class::cast)
- .collect(onlyElement());
+ .collect(MoreCollectors.onlyElement());
assertThat(attempt.getExecutionInfo().getStrategy()).isEqualTo("test");
- assertThat(attempt.getExecutionInfo().getHostname()).isEmpty();
+ assertThat(attempt.getExecutionInfo().getHostname()).isEqualTo("");
}
@Test
@@ -422,13 +393,13 @@
assertThat(attempts).hasSize(2);
TestAttempt failedAttempt = attempts.get(0);
assertThat(failedAttempt.getExecutionInfo().getStrategy()).isEqualTo("test");
- assertThat(failedAttempt.getExecutionInfo().getHostname()).isEmpty();
+ assertThat(failedAttempt.getExecutionInfo().getHostname()).isEqualTo("");
assertThat(failedAttempt.getStatus()).isEqualTo(TestStatus.FAILED);
assertThat(failedAttempt.getExecutionInfo().getCachedRemotely()).isFalse();
TestAttempt okAttempt = attempts.get(1);
assertThat(okAttempt.getStatus()).isEqualTo(TestStatus.PASSED);
assertThat(okAttempt.getExecutionInfo().getStrategy()).isEqualTo("test");
- assertThat(okAttempt.getExecutionInfo().getHostname()).isEmpty();
+ assertThat(okAttempt.getExecutionInfo().getHostname()).isEqualTo("");
}
@Test
@@ -479,10 +450,12 @@
assertThat(result.getData().getRunDurationMillis()).isEqualTo(10);
assertThat(result.getData().getTestTimesList()).containsExactly(10L);
TestAttempt attempt =
- storedEvents.getPosts().stream()
+ storedEvents
+ .getPosts()
+ .stream()
.filter(TestAttempt.class::isInstance)
.map(TestAttempt.class::cast)
- .collect(onlyElement());
+ .collect(MoreCollectors.onlyElement());
assertThat(attempt.getStatus()).isEqualTo(TestStatus.PASSED);
assertThat(attempt.getExecutionInfo().getStrategy()).isEqualTo("remote");
assertThat(attempt.getExecutionInfo().getHostname()).isEqualTo("a-remote-host");
@@ -537,12 +510,14 @@
assertThat(result.getData().getRunDurationMillis()).isEqualTo(10);
assertThat(result.getData().getTestTimesList()).containsExactly(10L);
TestAttempt attempt =
- storedEvents.getPosts().stream()
+ storedEvents
+ .getPosts()
+ .stream()
.filter(TestAttempt.class::isInstance)
.map(TestAttempt.class::cast)
- .collect(onlyElement());
+ .collect(MoreCollectors.onlyElement());
assertThat(attempt.getExecutionInfo().getStrategy()).isEqualTo("remote cache");
- assertThat(attempt.getExecutionInfo().getHostname()).isEmpty();
+ assertThat(attempt.getExecutionInfo().getHostname()).isEqualTo("");
}
@Test
@@ -1116,179 +1091,4 @@
TestResultData data = ((StandaloneFailedAttemptResult) failedResult).testResultData();
assertThat(data.getStatus()).isEqualTo(BlazeTestStatus.INCOMPLETE);
}
-
- @Test
- public void firstCreateTestRunnerSpawnThrows_testResultPosted() throws Exception {
- ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS;
- Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
- BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
- TestExecException testExecException =
- new TestExecException("test setup failure", BAD_FLAGS_FAILURE_DETAIL);
- TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot, testExecException);
-
- // setup a test action
- scratch.file("standalone/simple_test.sh", "this does not get executed, it is mocked out");
- scratch.file(
- "standalone/BUILD",
- "sh_test(",
- " name = 'simple_test',",
- " size = 'small',",
- " srcs = ['simple_test.sh'],",
- ")");
- TestRunnerAction testRunnerAction = getTestAction("//standalone:simple_test");
-
- ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
-
- // Actual StandaloneTestStrategy execution will fail the action due to the TestExecException,
- // while still posting the TestResult and TestAttempt.
- ActionExecutionException e =
- assertThrows(
- ActionExecutionException.class,
- () -> execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy));
- assertThat(e).hasCauseThat().isSameInstanceAs(testExecException);
-
- TestResult result = standaloneTestStrategy.postedResult;
- assertThat(result).isNotNull();
- assertThat(result.getData().getStatus()).isEqualTo(BlazeTestStatus.INCOMPLETE);
- assertThat(result.isCached()).isFalse();
- assertThat(result.getTestAction()).isSameInstanceAs(testRunnerAction);
- assertThat(result.getData().getTestPassed()).isFalse();
- TestAttempt attempt =
- storedEvents.getPosts().stream()
- .filter(TestAttempt.class::isInstance)
- .map(TestAttempt.class::cast)
- .collect(onlyElement());
- assertThat(attempt.getStatus()).isEqualTo(TestStatus.INCOMPLETE);
- assertThat(attempt.getAttempt()).isEqualTo(1);
- assertThat(attempt.isCachedLocally()).isFalse();
-
- verify(spawnStrategy, never()).beginExecution(any(), any());
- }
-
- @Test
- public void testSpawnThrowsExecExceptionOnFirstAttempt() throws Exception {
- ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
- Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
- BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
- TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
-
- // setup a test action
- scratch.file("standalone/simple_test.sh", "this does not get executed, it is mocked out");
- scratch.file(
- "standalone/BUILD",
- "sh_test(",
- " name = \"simple_test\",",
- " size = \"small\",",
- " srcs = [\"simple_test.sh\"],",
- " flaky = True,",
- ")");
- TestRunnerAction testRunnerAction = getTestAction("//standalone:simple_test");
-
- TestExecException testExecException =
- new TestExecException("test failed", BAD_FLAGS_FAILURE_DETAIL);
- when(spawnStrategy.beginExecution(any(), any()))
- .thenReturn(SpawnContinuation.failedWithExecException(testExecException));
-
- ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
-
- // actual StandaloneTestStrategy execution
- ActionExecutionException e =
- assertThrows(
- ActionExecutionException.class,
- () -> execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy));
- assertThat(e).hasCauseThat().isSameInstanceAs(testExecException);
-
- TestResult result = standaloneTestStrategy.postedResult;
- assertThat(result).isNotNull();
- assertThat(result.getData().getStatus()).isEqualTo(BlazeTestStatus.INCOMPLETE);
- assertThat(result.isCached()).isFalse();
- assertThat(result.getTestAction()).isSameInstanceAs(testRunnerAction);
- assertThat(result.getData().getTestPassed()).isFalse();
- TestAttempt attempt =
- storedEvents.getPosts().stream()
- .filter(TestAttempt.class::isInstance)
- .map(TestAttempt.class::cast)
- .collect(onlyElement());
- assertThat(attempt).isNotNull();
- assertThat(attempt.getStatus()).isEqualTo(TestStatus.INCOMPLETE);
- assertThat(attempt.getAttempt()).isEqualTo(1);
- assertThat(attempt.isCachedLocally()).isFalse();
- }
-
- @Test
- public void testSpawnThrowsExecExceptionOnSecondAttempt() throws Exception {
- ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
- // TODO(ulfjack): Update this test for split xml generation.
- executionOptions.splitXmlGeneration = false;
-
- Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
- BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
- TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
-
- // setup a test action
- scratch.file("standalone/simple_test.sh", "this does not get executed, it is mocked out");
- scratch.file(
- "standalone/BUILD",
- "sh_test(",
- " name = \"simple_test\",",
- " size = \"small\",",
- " srcs = [\"simple_test.sh\"],",
- " flaky = True,",
- ")");
- TestRunnerAction testRunnerAction = getTestAction("//standalone:simple_test");
-
- SpawnResult failSpawnResult =
- new SpawnResult.Builder()
- .setStatus(Status.NON_ZERO_EXIT)
- .setExitCode(1)
- .setFailureDetail(NON_ZERO_EXIT_DETAILS)
- .setWallTime(Duration.ofMillis(10))
- .setRunnerName("test")
- .build();
- TestExecException testExecException =
- new TestExecException("test failed", BAD_FLAGS_FAILURE_DETAIL);
- when(spawnStrategy.beginExecution(any(), any()))
- .thenReturn(
- SpawnContinuation.failedWithExecException(
- new SpawnExecException("test failed", failSpawnResult, false)))
- .thenReturn(SpawnContinuation.failedWithExecException(testExecException));
-
- ActionExecutionContext actionExecutionContext =
- new FakeActionExecutionContext(createTempOutErr(tmpDirRoot), spawnStrategy, binTools);
-
- // actual StandaloneTestStrategy execution
- ActionExecutionException e =
- assertThrows(
- ActionExecutionException.class,
- () -> execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy));
- assertThat(e).hasCauseThat().isSameInstanceAs(testExecException);
-
- TestResult result = standaloneTestStrategy.postedResult;
- assertThat(result).isNotNull();
- assertThat(result.getData().getStatus()).isEqualTo(BlazeTestStatus.INCOMPLETE);
- assertThat(result.isCached()).isFalse();
- assertThat(result.getTestAction()).isSameInstanceAs(testRunnerAction);
- assertThat(result.getData().getTestPassed()).isFalse();
- ImmutableList<TestAttempt> attempts =
- storedEvents.getPosts().stream()
- .filter(TestAttempt.class::isInstance)
- .map(TestAttempt.class::cast)
- .collect(ImmutableList.toImmutableList());
- assertThat(attempts).hasSize(2);
- TestAttempt failedAttempt = attempts.get(0);
- assertThat(failedAttempt.getExecutionInfo().getStrategy()).isEqualTo("test");
- assertThat(failedAttempt.getExecutionInfo().getHostname()).isEmpty();
- assertThat(failedAttempt.getExecutionInfo().getCachedRemotely()).isFalse();
- assertThat(failedAttempt.getStatus()).isEqualTo(TestStatus.FAILED);
- assertThat(failedAttempt.getAttempt()).isEqualTo(1);
- TestAttempt exceptionAttempt = attempts.get(1);
- assertThat(exceptionAttempt.getStatus()).isEqualTo(TestStatus.INCOMPLETE);
- assertThat(exceptionAttempt.getAttempt()).isEqualTo(2);
- assertThat(exceptionAttempt.isCachedLocally()).isFalse();
- }
}
diff --git a/src/test/shell/integration/build_event_stream_test.sh b/src/test/shell/integration/build_event_stream_test.sh
index bedc2ccc..c3b3108 100755
--- a/src/test/shell/integration/build_event_stream_test.sh
+++ b/src/test/shell/integration/build_event_stream_test.sh
@@ -1418,15 +1418,4 @@
expect_log 'used_heap_size_post_build: [1-9]'
}
-function test_failure_to_execute_status() {
- # The options --test_strategy=standalone --nobuild_runfile_manifests produce
- # a TestExecException at test execution time.
- bazel test --build_event_json_file=$TEST_log pkg:true \
- --test_strategy=standalone --nobuild_runfile_manifests \
- && fail "Expected failure"
-
- expect_not_log 'testResult.*"aborted"'
- expect_log 'testResult.*//pkg:true.*"INCOMPLETE"'
-}
-
run_suite "Integration tests for the build event stream"
