Move execute implementation up to TestRunnerAction
Instead, add methods to TestActionContext to construct a TestRunnerSpawn
and inform about test_keep_going.
As of this point, the high-level test execution process is now shared
between Bazel and Blaze. This is in preparation for implementing async
test execution.
PiperOrigin-RevId: 236107062
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 f36b4e0..928d170 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
@@ -26,16 +26,13 @@
* A context for the execution of test actions ({@link TestRunnerAction}).
*/
public interface TestActionContext extends ActionContext {
-
- /**
- * Executes the test command, directing standard out / err to {@code outErr}. The status of the
- * test should be communicated by posting a {@link TestResult} object to the eventbus.
- *
- * @return a list of SpawnResults created during execution of the test command, if any
- */
- List<SpawnResult> exec(TestRunnerAction action, ActionExecutionContext actionExecutionContext)
+ TestRunnerSpawn createTestRunnerSpawn(
+ TestRunnerAction testRunnerAction, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException;
+ /** Returns whether test_keep_going is enabled. */
+ boolean isTestKeepGoing();
+
/**
* Creates a cached test result.
*/
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 718133e..88e7861 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
@@ -31,15 +31,22 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier;
import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit;
+import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.actions.TestExecException;
import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.RunUnder;
+import com.google.devtools.build.lib.analysis.test.TestActionContext.FailedAttemptResult;
+import com.google.devtools.build.lib.analysis.test.TestActionContext.TestAttemptResult;
+import com.google.devtools.build.lib.analysis.test.TestActionContext.TestRunnerSpawn;
import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.ImmutableIterable;
+import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.util.Pair;
@@ -742,8 +749,19 @@
public ActionResult execute(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
TestActionContext context = actionExecutionContext.getContext(TestActionContext.class);
+ return execute(actionExecutionContext, context);
+ }
+
+ @VisibleForTesting
+ public ActionResult execute(
+ ActionExecutionContext actionExecutionContext, TestActionContext testActionContext)
+ throws ActionExecutionException, InterruptedException {
try {
- return ActionResult.create(context.exec(this, actionExecutionContext));
+ TestRunnerSpawn testRunnerSpawn =
+ testActionContext.createTestRunnerSpawn(this, actionExecutionContext);
+ return ActionResult.create(
+ runAttempts(
+ testRunnerSpawn, actionExecutionContext, testActionContext.isTestKeepGoing()));
} catch (ExecException e) {
throw e.toActionExecutionException(this);
} finally {
@@ -751,6 +769,67 @@
}
}
+ private List<SpawnResult> runAttempts(
+ TestRunnerSpawn testRunnerSpawn,
+ ActionExecutionContext actionExecutionContext,
+ boolean keepGoing)
+ throws ExecException, InterruptedException {
+ List<SpawnResult> spawnResults = new ArrayList<>();
+ runAttempts(
+ testRunnerSpawn, actionExecutionContext, keepGoing, spawnResults, new ArrayList<>());
+ return spawnResults;
+ }
+
+ private void runAttempts(
+ TestRunnerSpawn testRunnerSpawn,
+ ActionExecutionContext actionExecutionContext,
+ boolean keepGoing,
+ List<SpawnResult> spawnResults,
+ List<FailedAttemptResult> failedAttempts)
+ throws ExecException, InterruptedException {
+ try {
+ TestAttemptResult testProcessResult = testRunnerSpawn.execute();
+ spawnResults.addAll(testProcessResult.spawnResults());
+ int maxAttempts = testRunnerSpawn.getMaxAttempts(testProcessResult);
+ // Failed test retry loop.
+ for (int attempt = 1; attempt < maxAttempts && !testProcessResult.hasPassed(); attempt++) {
+ failedAttempts.add(
+ testRunnerSpawn.finalizeFailedTestAttempt(
+ testProcessResult, failedAttempts.size() + 1));
+
+ testProcessResult = testRunnerSpawn.execute();
+ spawnResults.addAll(testProcessResult.spawnResults());
+ }
+
+ TestRunnerSpawn fallbackRunner;
+ if (!testProcessResult.hasPassed()
+ && (fallbackRunner = testRunnerSpawn.getFallbackRunner()) != null) {
+ // All attempts failed and fallback is enabled.
+ failedAttempts.add(
+ testRunnerSpawn.finalizeFailedTestAttempt(
+ testProcessResult, failedAttempts.size() + 1));
+ runAttempts(
+ fallbackRunner, actionExecutionContext, keepGoing, spawnResults, failedAttempts);
+ } else {
+ testRunnerSpawn.finalizeTest(testProcessResult, failedAttempts);
+
+ if (!keepGoing && !testProcessResult.hasPassed()) {
+ throw new TestExecException("Test failed: aborting");
+ }
+ }
+ } catch (IOException e) {
+ // Print the stack trace, otherwise the unexpected I/O error is hard to diagnose.
+ // A stack trace could help with bugs like https://github.com/bazelbuild/bazel/issues/4924
+ StringBuilder sb = new StringBuilder();
+ sb.append("Caught I/O exception: ").append(e.getMessage());
+ for (Object s : e.getStackTrace()) {
+ sb.append("\n\t").append(s);
+ }
+ actionExecutionContext.getEventHandler().handle(Event.error(sb.toString()));
+ throw new EnvironmentalExecException("unexpected I/O exception", e);
+ }
+ }
+
@Override
public String getMnemonic() {
return MNEMONIC;
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 6885355..63c384d 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
@@ -91,14 +91,7 @@
}
@Override
- public List<SpawnResult> exec(
- TestRunnerAction action, ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException {
- TestRunnerSpawn testRunnerSpawn = createTestRunnerSpawn(action, actionExecutionContext);
- return runAttempts(testRunnerSpawn, actionExecutionContext);
- }
-
- protected TestRunnerSpawn createTestRunnerSpawn(
+ public TestRunnerSpawn createTestRunnerSpawn(
TestRunnerAction action, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
Path execRoot = actionExecutionContext.getExecRoot();
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index efb31ea..c936640 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -23,9 +23,7 @@
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
-import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
-import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.TestExecException;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -52,7 +50,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.time.Duration;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
@@ -128,64 +125,8 @@
}
@Override
- public abstract List<SpawnResult> exec(
- TestRunnerAction action, ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException;
-
- protected List<SpawnResult> runAttempts(
- TestRunnerSpawn testRunnerSpawn, ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException {
- List<SpawnResult> spawnResults = new ArrayList<>();
- runAttempts(testRunnerSpawn, actionExecutionContext, spawnResults, new ArrayList<>());
- return spawnResults;
- }
-
- private void runAttempts(
- TestRunnerSpawn testRunnerSpawn,
- ActionExecutionContext actionExecutionContext,
- List<SpawnResult> spawnResults,
- List<FailedAttemptResult> failedAttempts)
- throws ExecException, InterruptedException {
- try {
- TestAttemptResult testProcessResult = testRunnerSpawn.execute();
- spawnResults.addAll(testProcessResult.spawnResults());
- int maxAttempts = testRunnerSpawn.getMaxAttempts(testProcessResult);
- // Failed test retry loop.
- for (int attempt = 1; attempt < maxAttempts && !testProcessResult.hasPassed(); attempt++) {
- failedAttempts.add(
- testRunnerSpawn.finalizeFailedTestAttempt(
- testProcessResult, failedAttempts.size() + 1));
-
- testProcessResult = testRunnerSpawn.execute();
- spawnResults.addAll(testProcessResult.spawnResults());
- }
-
- TestRunnerSpawn fallbackRunner;
- if (!testProcessResult.hasPassed()
- && (fallbackRunner = testRunnerSpawn.getFallbackRunner()) != null) {
- // All attempts failed and fallback is enabled.
- failedAttempts.add(
- testRunnerSpawn.finalizeFailedTestAttempt(
- testProcessResult, failedAttempts.size() + 1));
- runAttempts(fallbackRunner, actionExecutionContext, spawnResults, failedAttempts);
- } else {
- testRunnerSpawn.finalizeTest(testProcessResult, failedAttempts);
-
- if (!executionOptions.testKeepGoing && !testProcessResult.hasPassed()) {
- throw new TestExecException("Test failed: aborting");
- }
- }
- } catch (IOException e) {
- // Print the stack trace, otherwise the unexpected I/O error is hard to diagnose.
- // A stack trace could help with bugs like https://github.com/bazelbuild/bazel/issues/4924
- StringBuilder sb = new StringBuilder();
- sb.append("Caught I/O exception: ").append(e.getMessage());
- for (Object s : e.getStackTrace()) {
- sb.append("\n\t").append(s);
- }
- actionExecutionContext.getEventHandler().handle(Event.error(sb.toString()));
- throw new EnvironmentalExecException("unexpected I/O exception", e);
- }
+ public final boolean isTestKeepGoing() {
+ return executionOptions.testKeepGoing;
}
/**
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 66e2576..99963e3 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
@@ -16,14 +16,12 @@
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
-import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.analysis.test.TestActionContext;
import com.google.devtools.build.lib.analysis.test.TestResult;
import com.google.devtools.build.lib.analysis.test.TestRunnerAction;
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
@@ -41,10 +39,15 @@
}
@Override
- public List<SpawnResult> exec(
- TestRunnerAction action, ActionExecutionContext actionExecutionContext)
+ public TestRunnerSpawn createTestRunnerSpawn(
+ TestRunnerAction testRunnerAction, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
- return parent.exec(action, actionExecutionContext);
+ return parent.createTestRunnerSpawn(testRunnerAction, actionExecutionContext);
+ }
+
+ @Override
+ public boolean isTestKeepGoing() {
+ return parent.isTestKeepGoing();
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
index d757799..322f6d9 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnActionContextMapsTest.java
@@ -35,7 +35,6 @@
import com.google.devtools.build.lib.util.RegexFilter.RegexFilterConverter;
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;
import org.junit.Before;
import org.junit.Test;
@@ -162,15 +161,19 @@
@ExecutionStrategy(contextType = TestActionContext.class, name = "actest")
private static class ACTest implements TestActionContext {
@Override
- public List<SpawnResult> exec(
- TestRunnerAction action, ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException {
+ public TestRunnerSpawn createTestRunnerSpawn(
+ TestRunnerAction testRunnerAction, ActionExecutionContext actionExecutionContext) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean isTestKeepGoing() {
throw new UnsupportedOperationException();
}
@Override
public TestResult newCachedTestResult(
- Path execRoot, TestRunnerAction action, TestResultData cached) throws IOException {
+ Path execRoot, TestRunnerAction action, TestResultData cached) {
throw new UnsupportedOperationException();
}
}
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 5ea50c6..28e636d 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
@@ -26,6 +26,7 @@
import com.google.common.collect.MoreCollectors;
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Artifact;
@@ -34,6 +35,7 @@
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.SpawnResult.Status;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.test.TestActionContext;
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;
@@ -153,6 +155,14 @@
return action;
}
+ private List<SpawnResult> execute(
+ TestRunnerAction testRunnerAction,
+ ActionExecutionContext actionExecutionContext,
+ TestActionContext testActionContext)
+ throws ActionExecutionException, InterruptedException {
+ return testRunnerAction.execute(actionExecutionContext, testActionContext).spawnResults();
+ }
+
@Test
public void testRunTestOnce() throws Exception {
ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS;
@@ -185,7 +195,7 @@
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
- standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext);
+ execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
assertThat(spawnResults).contains(expectedSpawnResult);
TestResult result = standaloneTestStrategy.postedResult;
@@ -253,7 +263,7 @@
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
- standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext);
+ execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
assertThat(spawnResults).containsExactly(failSpawnResult, passSpawnResult).inOrder();
@@ -319,7 +329,7 @@
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
- standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext);
+ execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
assertThat(spawnResults).contains(expectedSpawnResult);
@@ -377,7 +387,7 @@
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
- standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext);
+ execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
// check that the rigged SpawnResult was returned
assertThat(spawnResults).contains(expectedSpawnResult);
@@ -460,7 +470,7 @@
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
- standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext);
+ execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
// check that the rigged SpawnResult was returned
assertThat(spawnResults).contains(expectedSpawnResult);
@@ -558,7 +568,7 @@
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
- standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext);
+ execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
// check that the rigged SpawnResult was returned
assertThat(spawnResults).containsExactly(testSpawnResult, xmlGeneratorSpawnResult);
@@ -613,7 +623,7 @@
// actual StandaloneTestStrategy execution
List<SpawnResult> spawnResults =
- standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext);
+ execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
// check that the rigged SpawnResult was returned
assertThat(spawnResults).contains(expectedSpawnResult);
