Compute the list of test output files in TestRunnerAction
This allows us to slightly simplify the renaming logic in StandaloneTestStrategy.
PiperOrigin-RevId: 190645674
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestResult.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestResult.java
index 39cbcdb..b48649a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestResult.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestResult.java
@@ -15,10 +15,7 @@
package com.google.devtools.build.lib.analysis.test;
import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.test.TestRunnerAction.ResolvedPaths;
-import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -67,42 +64,6 @@
return status == BlazeTestStatus.PASSED || status == BlazeTestStatus.FLAKY;
}
- public static ImmutableList<Pair<String, Path>> testOutputsFromPaths(
- ResolvedPaths resolvedPaths) {
- ImmutableList.Builder<Pair<String, Path>> builder = new ImmutableList.Builder<>();
- if (resolvedPaths.getXmlOutputPath().exists()) {
- builder.add(Pair.of(TestFileNameConstants.TEST_XML, resolvedPaths.getXmlOutputPath()));
- }
- if (resolvedPaths.getSplitLogsPath().exists()) {
- builder.add(Pair.of(TestFileNameConstants.SPLIT_LOGS, resolvedPaths.getSplitLogsPath()));
- }
- if (resolvedPaths.getTestWarningsPath().exists()) {
- builder.add(Pair.of(TestFileNameConstants.TEST_WARNINGS,
- resolvedPaths.getTestWarningsPath()));
- }
- if (resolvedPaths.getUndeclaredOutputsZipPath().exists()) {
- builder.add(Pair.of(TestFileNameConstants.UNDECLARED_OUTPUTS_ZIP,
- resolvedPaths.getUndeclaredOutputsZipPath()));
- }
- if (resolvedPaths.getUndeclaredOutputsManifestPath().exists()) {
- builder.add(Pair.of(TestFileNameConstants.UNDECLARED_OUTPUTS_MANIFEST,
- resolvedPaths.getUndeclaredOutputsManifestPath()));
- }
- if (resolvedPaths.getUndeclaredOutputsAnnotationsPath().exists()) {
- builder.add(Pair.of(TestFileNameConstants.UNDECLARED_OUTPUTS_ANNOTATIONS,
- resolvedPaths.getUndeclaredOutputsAnnotationsPath()));
- }
- if (resolvedPaths.getUnusedRunfilesLogPath().exists()) {
- builder.add(Pair.of(TestFileNameConstants.UNUSED_RUNFILES_LOG,
- resolvedPaths.getUnusedRunfilesLogPath()));
- }
- if (resolvedPaths.getInfrastructureFailureFile().exists()) {
- builder.add(Pair.of(TestFileNameConstants.TEST_INFRASTRUCTURE_FAILURE,
- resolvedPaths.getInfrastructureFailureFile()));
- }
- return builder.build();
- }
-
/**
* @return The test action.
*/
@@ -186,17 +147,6 @@
* "test.log").
*/
public Collection<Pair<String, Path>> getFiles() {
- ImmutableList.Builder<Pair<String, Path>> builder = new ImmutableList.Builder<>();
- if (testAction.getTestLog().getPath().exists()) {
- builder.add(Pair.of(TestFileNameConstants.TEST_LOG, testAction.getTestLog().getPath()));
- }
- if (testAction.getCoverageData() != null && testAction.getCoverageData().getPath().exists()) {
- builder.add(
- Pair.of(TestFileNameConstants.TEST_COVERAGE, testAction.getCoverageData().getPath()));
- }
- if (execRoot != null) {
- builder.addAll(testOutputsFromPaths(testAction.resolve(execRoot)));
- }
- return builder.build();
+ return testAction.getTestOutputsMapping(execRoot);
}
}
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 22638c7..336ca69 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
@@ -35,10 +35,12 @@
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.buildeventstream.TestFileNameConstants;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.ImmutableIterable;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.LoggingUtil;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -231,6 +233,55 @@
return outputs;
}
+ /**
+ * Returns the list of mappings from file name constants to output files. This method checks the
+ * file system for existence of these output files, so it must only be used after test execution.
+ */
+ // TODO(ulfjack): Instead of going to local disk here, use SpawnResult (add list of files there).
+ public ImmutableList<Pair<String, Path>> getTestOutputsMapping(Path execRoot) {
+ ImmutableList.Builder<Pair<String, Path>> builder = ImmutableList.builder();
+ if (getTestLog().getPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.TEST_LOG, getTestLog().getPath()));
+ }
+ if (getCoverageData() != null && getCoverageData().getPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.TEST_COVERAGE, getCoverageData().getPath()));
+ }
+ if (execRoot != null) {
+ ResolvedPaths resolvedPaths = resolve(execRoot);
+ if (resolvedPaths.getXmlOutputPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.TEST_XML, resolvedPaths.getXmlOutputPath()));
+ }
+ if (resolvedPaths.getSplitLogsPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.SPLIT_LOGS, resolvedPaths.getSplitLogsPath()));
+ }
+ if (resolvedPaths.getTestWarningsPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.TEST_WARNINGS,
+ resolvedPaths.getTestWarningsPath()));
+ }
+ if (resolvedPaths.getUndeclaredOutputsZipPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.UNDECLARED_OUTPUTS_ZIP,
+ resolvedPaths.getUndeclaredOutputsZipPath()));
+ }
+ if (resolvedPaths.getUndeclaredOutputsManifestPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.UNDECLARED_OUTPUTS_MANIFEST,
+ resolvedPaths.getUndeclaredOutputsManifestPath()));
+ }
+ if (resolvedPaths.getUndeclaredOutputsAnnotationsPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.UNDECLARED_OUTPUTS_ANNOTATIONS,
+ resolvedPaths.getUndeclaredOutputsAnnotationsPath()));
+ }
+ if (resolvedPaths.getUnusedRunfilesLogPath().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.UNUSED_RUNFILES_LOG,
+ resolvedPaths.getUnusedRunfilesLogPath()));
+ }
+ if (resolvedPaths.getInfrastructureFailureFile().exists()) {
+ builder.add(Pair.of(TestFileNameConstants.TEST_INFRASTRUCTURE_FAILURE,
+ resolvedPaths.getInfrastructureFailureFile()));
+ }
+ }
+ return builder.build();
+ }
+
@Override
protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp)
throws CommandLineExpansionException {
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 c3828d3..aa83718 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
@@ -163,21 +163,7 @@
workingDirectory);
}
processLastTestAttempt(attempt, dataBuilder, standaloneTestResult.testResultData());
- ImmutableList.Builder<Pair<String, Path>> testOutputsBuilder = new ImmutableList.Builder<>();
- if (actionExecutionContext.getInputPath(action.getTestLog()).exists()) {
- testOutputsBuilder.add(
- Pair.of(
- TestFileNameConstants.TEST_LOG,
- actionExecutionContext.getInputPath(action.getTestLog())));
- }
- if (action.getCoverageData() != null
- && actionExecutionContext.getInputPath(action.getCoverageData()).exists()) {
- testOutputsBuilder.add(
- Pair.of(
- TestFileNameConstants.TEST_COVERAGE,
- actionExecutionContext.getInputPath(action.getCoverageData())));
- }
- testOutputsBuilder.addAll(TestResult.testOutputsFromPaths(resolvedPaths));
+ ImmutableList<Pair<String, Path>> testOutputs = action.getTestOutputsMapping(execRoot);
actionExecutionContext
.getEventBus()
.post(
@@ -187,7 +173,7 @@
standaloneTestResult.testResultData().getStatus(),
standaloneTestResult.testResultData().getStartTimeMillisEpoch(),
standaloneTestResult.testResultData().getRunDurationMillis(),
- testOutputsBuilder.build(),
+ testOutputs,
standaloneTestResult.testResultData().getWarningList(),
true));
finalizeTest(actionExecutionContext, action, dataBuilder.build());
@@ -217,39 +203,33 @@
attemptsDir.createDirectory();
String attemptPrefix = "attempt_" + attempt;
Path testLog = attemptsDir.getChild(attemptPrefix + ".log");
- if (actionExecutionContext.getInputPath(action.getTestLog()).exists()) {
- actionExecutionContext.getInputPath(action.getTestLog()).renameTo(testLog);
- testOutputsBuilder.add(Pair.of(TestFileNameConstants.TEST_LOG, testLog));
- }
- if (action.getCoverageData() != null
- && actionExecutionContext.getInputPath(action.getCoverageData()).exists()) {
- testOutputsBuilder.add(
- Pair.of(
- TestFileNameConstants.TEST_COVERAGE,
- actionExecutionContext.getInputPath(action.getCoverageData())));
- }
// Get the normal test output paths, and then update them to use "attempt_N" names, and
// attemptDir, before adding them to the outputs.
- ResolvedPaths resolvedPaths = action.resolve(actionExecutionContext.getExecRoot());
- ImmutableList<Pair<String, Path>> testOutputs = TestResult.testOutputsFromPaths(resolvedPaths);
+ ImmutableList<Pair<String, Path>> testOutputs =
+ action.getTestOutputsMapping(actionExecutionContext.getExecRoot());
for (Pair<String, Path> testOutput : testOutputs) {
// e.g. /testRoot/test.dir/file, an example we follow throughout this loop's comments.
Path testOutputPath = testOutput.getSecond();
+ Path destinationPath;
+ if (testOutput.getFirst().equals(TestFileNameConstants.TEST_LOG)) {
+ // The rename rules for the test log are different than for all the other files.
+ destinationPath = testLog;
+ } else {
+ // e.g. test.dir/file
+ PathFragment relativeToTestDirectory = testOutputPath.relativeTo(testRoot);
- // e.g. test.dir/file
- PathFragment relativeToTestDirectory = testOutputPath.relativeTo(testRoot);
+ // e.g. attempt_1.dir/file
+ String destinationPathFragmentStr =
+ relativeToTestDirectory.getSafePathString().replaceFirst("test", attemptPrefix);
+ PathFragment destinationPathFragment = PathFragment.create(destinationPathFragmentStr);
- // e.g. attempt_1.dir/file
- String destinationPathFragmentStr =
- relativeToTestDirectory.getSafePathString().replaceFirst("test", attemptPrefix);
- PathFragment destinationPathFragment = PathFragment.create(destinationPathFragmentStr);
+ // e.g. /attemptsDir/attempt_1.dir/file
+ destinationPath = attemptsDir.getRelative(destinationPathFragment);
+ destinationPath.getParentDirectory().createDirectory();
+ }
- // e.g. /attemptsDir/attempt_1.dir/file
- Path destinationPath = attemptsDir.getRelative(destinationPathFragment);
- destinationPath.getParentDirectory().createDirectory();
-
- // Copy to the destination.
+ // Move to the destination.
testOutputPath.renameTo(destinationPath);
testOutputsBuilder.add(Pair.of(testOutput.getFirst(), destinationPath));
