Remove implementations of TestAttemptContinuation.
Now TestStrategy.beginExecution() will always return a "done" continuation.
RELNOTES: None.
PiperOrigin-RevId: 491937994
Change-Id: I54f8256339b150d2912e84375578b3a5b9a0621f
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 0ebeaa3..ba16e4e 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
@@ -22,7 +22,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.io.ByteStreams;
-import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
@@ -33,7 +32,6 @@
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.SimpleSpawn;
import com.google.devtools.build.lib.actions.Spawn;
-import com.google.devtools.build.lib.actions.SpawnContinuation;
import com.google.devtools.build.lib.actions.SpawnMetrics;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.TestExecException;
@@ -70,7 +68,6 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
-import javax.annotation.Nullable;
/** Runs TestRunnerAction actions. */
// TODO(bazel-team): add tests for this strategy.
@@ -303,7 +300,7 @@
Spawn spawn,
ActionExecutionContext actionExecutionContext,
Path execRoot)
- throws IOException, InterruptedException {
+ throws ExecException, IOException, InterruptedException {
ResolvedPaths resolvedPaths = testAction.resolve(execRoot);
Path out = actionExecutionContext.getInputPath(testAction.getTestLog());
Path err = resolvedPaths.getTestStderr();
@@ -317,32 +314,16 @@
long startTimeMillis = actionExecutionContext.getClock().currentTimeMillis();
SpawnStrategyResolver resolver = actionExecutionContext.getContext(SpawnStrategyResolver.class);
- SpawnContinuation spawnContinuation;
- try {
- ImmutableList<SpawnResult> spawnResults =
- resolver.exec(spawn, actionExecutionContext.withFileOutErr(testOutErr));
- spawnContinuation = SpawnContinuation.immediate(spawnResults);
- } catch (InterruptedException e) {
- if (streamed != null) {
- streamed.close();
- }
- testOutErr.close();
- throw e;
- } catch (ExecException e) {
- spawnContinuation = SpawnContinuation.failedWithExecException(e);
- }
- return new BazelTestAttemptContinuation(
+ return runTestAttempt(
testAction,
actionExecutionContext,
spawn,
+ resolver,
resolvedPaths,
testOutErr,
streamed,
- startTimeMillis,
- spawnContinuation,
- /* testResultDataBuilder= */ null,
- /* spawnResults= */ null);
+ startTimeMillis);
}
private static void appendCoverageLog(FileOutErr coverageOutErr, FileOutErr outErr)
@@ -598,7 +579,8 @@
}
@Override
- public TestAttemptContinuation beginExecution() throws InterruptedException, IOException {
+ public TestAttemptContinuation beginExecution()
+ throws InterruptedException, IOException, ExecException {
prepareFileSystem(testAction, execRoot, tmpDir, workingDirectory);
return beginTestAttempt(testAction, spawn, actionExecutionContext, execRoot);
}
@@ -650,267 +632,214 @@
.build());
}
- private final class BazelTestAttemptContinuation extends TestAttemptContinuation {
- private final TestRunnerAction testAction;
- private final ActionExecutionContext actionExecutionContext;
- private final Spawn spawn;
- private final ResolvedPaths resolvedPaths;
- private final FileOutErr fileOutErr;
- private final Closeable streamed;
- private final long startTimeMillis;
- private final SpawnContinuation spawnContinuation;
- private TestResultData.Builder testResultDataBuilder;
- private ImmutableList<SpawnResult> spawnResults;
+ private TestAttemptContinuation runTestAttempt(
+ TestRunnerAction testAction,
+ ActionExecutionContext actionExecutionContext,
+ Spawn spawn,
+ SpawnStrategyResolver resolver,
+ ResolvedPaths resolvedPaths,
+ FileOutErr fileOutErr,
+ Closeable streamed,
+ long startTimeMillis)
+ throws InterruptedException, ExecException, IOException {
- BazelTestAttemptContinuation(
- TestRunnerAction testAction,
- ActionExecutionContext actionExecutionContext,
- Spawn spawn,
- ResolvedPaths resolvedPaths,
- FileOutErr fileOutErr,
- Closeable streamed,
- long startTimeMillis,
- SpawnContinuation spawnContinuation,
- TestResultData.Builder testResultDataBuilder,
- ImmutableList<SpawnResult> spawnResults) {
- this.testAction = testAction;
- this.actionExecutionContext = actionExecutionContext;
- this.spawn = spawn;
- this.resolvedPaths = resolvedPaths;
- this.fileOutErr = fileOutErr;
- this.streamed = streamed;
- this.startTimeMillis = startTimeMillis;
- this.spawnContinuation = spawnContinuation;
- this.testResultDataBuilder = testResultDataBuilder;
- this.spawnResults = spawnResults;
+ ImmutableList<SpawnResult> spawnResults;
+
+ // We have two protos to represent test attempts:
+ // 1. com.google.devtools.build.lib.view.test.TestStatus.TestResultData represents both
+ // failed attempts and finished tests. Bazel stores this to disk to persist cached test
+ // result information across server restarts.
+ // 2. com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.TestResult
+ // represents only individual attempts (failed or not). Bazel reports this as an event to
+ // the Build Event Protocol, but never saves it to disk.
+ //
+ // The TestResult proto is always constructed from a TestResultData instance, either one
+ // that is created right here, or one that is read back from disk.
+ TestResultData.Builder testResultDataBuilder;
+ try {
+ spawnResults = resolver.exec(spawn, actionExecutionContext.withFileOutErr(fileOutErr));
+ testResultDataBuilder = TestResultData.newBuilder();
+ testResultDataBuilder.setCachable(true).setTestPassed(true).setStatus(BlazeTestStatus.PASSED);
+ } catch (SpawnExecException e) {
+ if (e.isCatastrophic()) {
+ closeSuppressed(e, streamed);
+ closeSuppressed(e, fileOutErr);
+ throw e;
+ }
+ if (!e.getSpawnResult().setupSuccess()) {
+ closeSuppressed(e, streamed);
+ closeSuppressed(e, fileOutErr);
+ // Rethrow as the test could not be run and thus there's no point in retrying.
+ throw e;
+ }
+ spawnResults = ImmutableList.of(e.getSpawnResult());
+ testResultDataBuilder = TestResultData.newBuilder();
+ testResultDataBuilder
+ .setCachable(e.getSpawnResult().status().isConsideredUserError())
+ .setTestPassed(false)
+ .setStatus(e.hasTimedOut() ? BlazeTestStatus.TIMEOUT : BlazeTestStatus.FAILED);
+ } catch (InterruptedException e) {
+ closeSuppressed(e, streamed);
+ closeSuppressed(e, fileOutErr);
+ throw e;
}
+ long endTimeMillis = actionExecutionContext.getClock().currentTimeMillis();
- @Nullable
- @Override
- public ListenableFuture<?> getFuture() {
- return spawnContinuation.getFuture();
- }
+ // SpawnActionContext guarantees the first entry to correspond to the spawn passed in (there
+ // may be additional entries due to tree artifact handling).
+ SpawnResult primaryResult = spawnResults.get(0);
- @Override
- public TestAttemptContinuation execute()
- throws InterruptedException, ExecException, IOException {
+ // The SpawnResult of a remotely cached or remotely executed action may not have walltime
+ // set. We fall back to the time measured here for backwards compatibility.
+ long durationMillis = endTimeMillis - startTimeMillis;
+ durationMillis =
+ primaryResult.getWallTime().orElse(Duration.ofMillis(durationMillis)).toMillis();
- if (testResultDataBuilder == null) {
- // We have two protos to represent test attempts:
- // 1. com.google.devtools.build.lib.view.test.TestStatus.TestResultData represents both
- // failed attempts and finished tests. Bazel stores this to disk to persist cached test
- // result information across server restarts.
- // 2. com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.TestResult
- // represents only individual attempts (failed or not). Bazel reports this as an event to
- // the Build Event Protocol, but never saves it to disk.
- //
- // The TestResult proto is always constructed from a TestResultData instance, either one
- // that is created right here, or one that is read back from disk.
- TestResultData.Builder builder = null;
- ImmutableList<SpawnResult> spawnResults;
- try {
- SpawnContinuation nextContinuation = spawnContinuation.execute();
- if (!nextContinuation.isDone()) {
- return new BazelTestAttemptContinuation(
- testAction,
- actionExecutionContext,
- spawn,
- resolvedPaths,
- fileOutErr,
- streamed,
- startTimeMillis,
- nextContinuation,
- builder,
- /* spawnResults= */ null);
- }
- spawnResults = nextContinuation.get();
- builder = TestResultData.newBuilder();
- builder.setCachable(true).setTestPassed(true).setStatus(BlazeTestStatus.PASSED);
- } catch (SpawnExecException e) {
- if (e.isCatastrophic()) {
- closeSuppressed(e, streamed);
- closeSuppressed(e, fileOutErr);
- throw e;
- }
- if (!e.getSpawnResult().setupSuccess()) {
- closeSuppressed(e, streamed);
- closeSuppressed(e, fileOutErr);
- // Rethrow as the test could not be run and thus there's no point in retrying.
- throw e;
- }
- spawnResults = ImmutableList.of(e.getSpawnResult());
- builder = TestResultData.newBuilder();
- builder
- .setCachable(e.getSpawnResult().status().isConsideredUserError())
- .setTestPassed(false)
- .setStatus(e.hasTimedOut() ? BlazeTestStatus.TIMEOUT : BlazeTestStatus.FAILED);
- } catch (InterruptedException e) {
+ testResultDataBuilder
+ .setStartTimeMillisEpoch(startTimeMillis)
+ .addTestTimes(durationMillis)
+ .addTestProcessTimes(durationMillis)
+ .setRunDurationMillis(durationMillis)
+ .setHasCoverage(testAction.isCoverageMode());
+
+ if (testAction.isCoverageMode() && testAction.getSplitCoveragePostProcessing()) {
+ if (testAction.getCoverageDirectoryTreeArtifact() == null) {
+ // Otherwise we'll get a NPE https://github.com/bazelbuild/bazel/issues/13185
+ TestExecException e =
+ createTestExecException(
+ TestAction.Code.LOCAL_TEST_PREREQ_UNMET,
+ "coverageDirectoryTreeArtifact is null:"
+ + " --experimental_split_coverage_postprocessing depends on"
+ + " --experimental_fetch_all_coverage_outputs being enabled");
+ closeSuppressed(e, streamed);
+ closeSuppressed(e, fileOutErr);
+ throw e;
+ }
+ actionExecutionContext
+ .getMetadataHandler()
+ .getMetadata(testAction.getCoverageDirectoryTreeArtifact());
+ ImmutableSet<? extends ActionInput> expandedCoverageDir =
+ actionExecutionContext
+ .getMetadataHandler()
+ .getTreeArtifactChildren(
+ (SpecialArtifact) testAction.getCoverageDirectoryTreeArtifact());
+ Spawn coveragePostProcessingSpawn =
+ createCoveragePostProcessingSpawn(
+ actionExecutionContext,
+ testAction,
+ ImmutableList.copyOf(expandedCoverageDir),
+ tmpDirRoot,
+ executionOptions.splitXmlGeneration);
+ SpawnStrategyResolver spawnStrategyResolver =
+ actionExecutionContext.getContext(SpawnStrategyResolver.class);
+
+ Path testRoot =
+ actionExecutionContext.getInputPath(testAction.getTestLog()).getParentDirectory();
+
+ Path out = testRoot.getChild("coverage.log");
+ Path err = testRoot.getChild("coverage.err");
+ FileOutErr coverageOutErr = new FileOutErr(out, err);
+ ActionExecutionContext actionExecutionContextWithCoverageFileOutErr =
+ actionExecutionContext.withFileOutErr(coverageOutErr);
+
+ writeOutFile(coverageOutErr.getErrorPath(), coverageOutErr.getOutputPath());
+ appendCoverageLog(coverageOutErr, fileOutErr);
+ try {
+ spawnStrategyResolver.exec(
+ coveragePostProcessingSpawn, actionExecutionContextWithCoverageFileOutErr);
+ } catch (SpawnExecException e) {
+ if (e.isCatastrophic()) {
closeSuppressed(e, streamed);
closeSuppressed(e, fileOutErr);
throw e;
}
- long endTimeMillis = actionExecutionContext.getClock().currentTimeMillis();
-
- // SpawnActionContext guarantees the first entry to correspond to the spawn passed in (there
- // may be additional entries due to tree artifact handling).
- SpawnResult primaryResult = spawnResults.get(0);
-
- // The SpawnResult of a remotely cached or remotely executed action may not have walltime
- // set. We fall back to the time measured here for backwards compatibility.
- long durationMillis = endTimeMillis - startTimeMillis;
- durationMillis =
- primaryResult.getWallTime().orElse(Duration.ofMillis(durationMillis)).toMillis();
-
- builder
- .setStartTimeMillisEpoch(startTimeMillis)
- .addTestTimes(durationMillis)
- .addTestProcessTimes(durationMillis)
- .setRunDurationMillis(durationMillis)
- .setHasCoverage(testAction.isCoverageMode());
-
- if (testAction.isCoverageMode() && testAction.getSplitCoveragePostProcessing()) {
- if (testAction.getCoverageDirectoryTreeArtifact() == null) {
- // Otherwise we'll get a NPE https://github.com/bazelbuild/bazel/issues/13185
- TestExecException e =
- createTestExecException(
- TestAction.Code.LOCAL_TEST_PREREQ_UNMET,
- "coverageDirectoryTreeArtifact is null:"
- + " --experimental_split_coverage_postprocessing depends on"
- + " --experimental_fetch_all_coverage_outputs being enabled");
- closeSuppressed(e, streamed);
- closeSuppressed(e, fileOutErr);
- throw e;
- }
- actionExecutionContext
- .getMetadataHandler()
- .getMetadata(testAction.getCoverageDirectoryTreeArtifact());
- ImmutableSet<? extends ActionInput> expandedCoverageDir =
- actionExecutionContext
- .getMetadataHandler()
- .getTreeArtifactChildren(
- (SpecialArtifact) testAction.getCoverageDirectoryTreeArtifact());
- Spawn coveragePostProcessingSpawn =
- createCoveragePostProcessingSpawn(
- actionExecutionContext,
- testAction,
- ImmutableList.copyOf(expandedCoverageDir),
- tmpDirRoot,
- executionOptions.splitXmlGeneration);
- SpawnStrategyResolver spawnStrategyResolver =
- actionExecutionContext.getContext(SpawnStrategyResolver.class);
-
- Path testRoot =
- actionExecutionContext.getInputPath(testAction.getTestLog()).getParentDirectory();
-
- Path out = testRoot.getChild("coverage.log");
- Path err = testRoot.getChild("coverage.err");
- FileOutErr coverageOutErr = new FileOutErr(out, err);
- ActionExecutionContext actionExecutionContextWithCoverageFileOutErr =
- actionExecutionContext.withFileOutErr(coverageOutErr);
-
- writeOutFile(coverageOutErr.getErrorPath(), coverageOutErr.getOutputPath());
- appendCoverageLog(coverageOutErr, fileOutErr);
- try {
- spawnStrategyResolver.exec(
- coveragePostProcessingSpawn, actionExecutionContextWithCoverageFileOutErr);
- } catch (SpawnExecException e) {
- if (e.isCatastrophic()) {
- closeSuppressed(e, streamed);
- closeSuppressed(e, fileOutErr);
- throw e;
- }
- if (!e.getSpawnResult().setupSuccess()) {
- closeSuppressed(e, streamed);
- closeSuppressed(e, fileOutErr);
- // Rethrow as the test could not be run and thus there's no point in retrying.
- throw e;
- }
- testResultDataBuilder
- .setCachable(e.getSpawnResult().status().isConsideredUserError())
- .setTestPassed(false)
- .setStatus(e.hasTimedOut() ? BlazeTestStatus.TIMEOUT : BlazeTestStatus.FAILED);
- } catch (ExecException | InterruptedException e) {
- closeSuppressed(e, streamed);
- closeSuppressed(e, fileOutErr);
- throw e;
- }
- }
-
- this.spawnResults = spawnResults;
- this.testResultDataBuilder = builder;
- }
-
- Verify.verify(
- !(testAction.isCoverageMode() && testAction.getSplitCoveragePostProcessing())
- || testAction.getCoverageData().getPath().exists());
- Verify.verifyNotNull(spawnResults);
- Verify.verifyNotNull(testResultDataBuilder);
-
- try {
- if (!fileOutErr.hasRecordedOutput()) {
- // Make sure that the test.log exists.Spaw
- FileSystemUtils.touchFile(fileOutErr.getOutputPath());
- }
- // Append any error output to the test.log. This is very rare.
- writeOutFile(fileOutErr.getErrorPath(), fileOutErr.getOutputPath());
- fileOutErr.close();
- if (streamed != null) {
- streamed.close();
- }
- } catch (IOException e) {
- throw new EnvironmentalExecException(e, Code.TEST_OUT_ERR_IO_EXCEPTION);
- }
-
- Path xmlOutputPath = resolvedPaths.getXmlOutputPath();
-
- // If the test did not create a test.xml, and --experimental_split_xml_generation is enabled,
- // then we run a separate action to create a test.xml from test.log. We do this as a spawn
- // rather than doing it locally in-process, as the test.log file may only exist remotely (when
- // remote execution is enabled), and we do not want to have to download it.
- if (executionOptions.splitXmlGeneration
- && fileOutErr.getOutputPath().exists()
- && !xmlOutputPath.exists()) {
- Spawn xmlGeneratingSpawn =
- createXmlGeneratingSpawn(testAction, spawn.getEnvironment(), spawnResults.get(0));
- SpawnStrategyResolver spawnStrategyResolver =
- actionExecutionContext.getContext(SpawnStrategyResolver.class);
- // We treat all failures to generate the test.xml here as catastrophic, and won't rerun
- // the test if this fails. We redirect the output to a temporary file.
- FileOutErr xmlSpawnOutErr = actionExecutionContext.getFileOutErr().childOutErr();
- try {
-
- ImmutableList<SpawnResult> xmlSpawnResults =
- spawnStrategyResolver.exec(
- xmlGeneratingSpawn, actionExecutionContext.withFileOutErr(xmlSpawnOutErr));
- this.spawnResults =
- ImmutableList.<SpawnResult>builder()
- .addAll(spawnResults)
- .addAll(xmlSpawnResults)
- .build();
- } catch (InterruptedException | ExecException e) {
- closeSuppressed(e, xmlSpawnOutErr);
+ if (!e.getSpawnResult().setupSuccess()) {
+ closeSuppressed(e, streamed);
+ closeSuppressed(e, fileOutErr);
+ // Rethrow as the test could not be run and thus there's no point in retrying.
throw e;
}
+ testResultDataBuilder
+ .setCachable(e.getSpawnResult().status().isConsideredUserError())
+ .setTestPassed(false)
+ .setStatus(e.hasTimedOut() ? BlazeTestStatus.TIMEOUT : BlazeTestStatus.FAILED);
+ } catch (ExecException | InterruptedException e) {
+ closeSuppressed(e, streamed);
+ closeSuppressed(e, fileOutErr);
+ throw e;
}
-
- TestCase details = parseTestResult(xmlOutputPath);
- if (details != null) {
- testResultDataBuilder.setTestCase(details);
- }
-
- BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo =
- extractExecutionInfo(spawnResults.get(0), testResultDataBuilder);
- StandaloneTestResult standaloneTestResult =
- StandaloneTestResult.builder()
- .setSpawnResults(spawnResults)
- // We return the TestResultData.Builder rather than the finished TestResultData
- // instance, as we may have to rename the output files in case the test needs to be
- // rerun (if it failed here _and_ is marked flaky _and_ the number of flaky attempts
- // is larger than 1).
- .setTestResultDataBuilder(testResultDataBuilder)
- .setExecutionInfo(executionInfo)
- .build();
- return TestAttemptContinuation.of(standaloneTestResult);
}
+
+ Verify.verify(
+ !(testAction.isCoverageMode() && testAction.getSplitCoveragePostProcessing())
+ || testAction.getCoverageData().getPath().exists());
+ Verify.verifyNotNull(spawnResults);
+ Verify.verifyNotNull(testResultDataBuilder);
+
+ try {
+ if (!fileOutErr.hasRecordedOutput()) {
+ // Make sure that the test.log exists.Spaw
+ FileSystemUtils.touchFile(fileOutErr.getOutputPath());
+ }
+ // Append any error output to the test.log. This is very rare.
+ writeOutFile(fileOutErr.getErrorPath(), fileOutErr.getOutputPath());
+ fileOutErr.close();
+ if (streamed != null) {
+ streamed.close();
+ }
+ } catch (IOException e) {
+ throw new EnvironmentalExecException(e, Code.TEST_OUT_ERR_IO_EXCEPTION);
+ }
+
+ Path xmlOutputPath = resolvedPaths.getXmlOutputPath();
+
+ // If the test did not create a test.xml, and --experimental_split_xml_generation is enabled,
+ // then we run a separate action to create a test.xml from test.log. We do this as a spawn
+ // rather than doing it locally in-process, as the test.log file may only exist remotely (when
+ // remote execution is enabled), and we do not want to have to download it.
+ if (executionOptions.splitXmlGeneration
+ && fileOutErr.getOutputPath().exists()
+ && !xmlOutputPath.exists()) {
+ Spawn xmlGeneratingSpawn =
+ createXmlGeneratingSpawn(testAction, spawn.getEnvironment(), spawnResults.get(0));
+ SpawnStrategyResolver spawnStrategyResolver =
+ actionExecutionContext.getContext(SpawnStrategyResolver.class);
+ // We treat all failures to generate the test.xml here as catastrophic, and won't rerun
+ // the test if this fails. We redirect the output to a temporary file.
+ FileOutErr xmlSpawnOutErr = actionExecutionContext.getFileOutErr().childOutErr();
+ try {
+
+ ImmutableList<SpawnResult> xmlSpawnResults =
+ spawnStrategyResolver.exec(
+ xmlGeneratingSpawn, actionExecutionContext.withFileOutErr(xmlSpawnOutErr));
+ spawnResults =
+ ImmutableList.<SpawnResult>builder()
+ .addAll(spawnResults)
+ .addAll(xmlSpawnResults)
+ .build();
+ } catch (InterruptedException | ExecException e) {
+ closeSuppressed(e, xmlSpawnOutErr);
+ throw e;
+ }
+ }
+
+ TestCase details = parseTestResult(xmlOutputPath);
+ if (details != null) {
+ testResultDataBuilder.setTestCase(details);
+ }
+
+ BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo =
+ extractExecutionInfo(spawnResults.get(0), testResultDataBuilder);
+ StandaloneTestResult standaloneTestResult =
+ StandaloneTestResult.builder()
+ .setSpawnResults(spawnResults)
+ // We return the TestResultData.Builder rather than the finished TestResultData
+ // instance, as we may have to rename the output files in case the test needs to be
+ // rerun (if it failed here _and_ is marked flaky _and_ the number of flaky attempts
+ // is larger than 1).
+ .setTestResultDataBuilder(testResultDataBuilder)
+ .setExecutionInfo(executionInfo)
+ .build();
+ return TestAttemptContinuation.of(standaloneTestResult);
}
}
