From 2f08a1805e33b6569a7ad489045e4a426a136315 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 21 Sep 2017 18:51:20 +0200 Subject: Include all test output files (e.g. undeclared outputs) for non-cached test executions and tests with multiple attempts. Previously, non-cached test execution would incorrectly only include test.log and test.xml. Implement this by unifying test output list building in a static TestResult method. For additional output files in multi-attempt tests, we follow the existing convention: Existing example: test.xml => test_attempts/attempt_N.xml New example (all new outputs done similarly): test.outputs/outputs.zip => test_attempts/attempt_N.outputs/outputs.zip RELNOTES: All test output files included for cached, uncached, and multiple attempt tests. PiperOrigin-RevId: 169556608 --- .../build/lib/analysis/test/TestResult.java | 67 ++++++++++++---------- .../build/lib/exec/StandaloneTestStrategy.java | 43 ++++++++++---- 2 files changed, 69 insertions(+), 41 deletions(-) (limited to 'src/main/java/com/google/devtools/build') 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 11f95a4656..98fbb420d5 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 @@ -67,6 +67,42 @@ public class TestResult { return status == BlazeTestStatus.PASSED || status == BlazeTestStatus.FLAKY; } + public static ImmutableList> testOutputsFromPaths( + ResolvedPaths resolvedPaths) { + ImmutableList.Builder> 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. */ @@ -155,36 +191,7 @@ public class TestResult { builder.add(Pair.of(TestFileNameConstants.TEST_LOG, testAction.getTestLog().getPath())); } if (execRoot != null) { - ResolvedPaths resolvedPaths = testAction.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.getSplitLogsPath())); - } - 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.getUndeclaredOutputsManifestPath())); - } - 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())); - } + builder.addAll(testOutputsFromPaths(testAction.resolve(execRoot))); } return builder.build(); } 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 52a9d21591..da0cec3982 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 @@ -33,6 +33,7 @@ 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.analysis.test.TestRunnerAction.ResolvedPaths; +import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; @@ -167,11 +168,10 @@ public class StandaloneTestStrategy extends TestStrategy { processLastTestAttempt(attempt, dataBuilder, data); ImmutableList.Builder> testOutputsBuilder = new ImmutableList.Builder<>(); if (action.getTestLog().getPath().exists()) { - testOutputsBuilder.add(Pair.of("test.log", action.getTestLog().getPath())); - } - if (resolvedPaths.getXmlOutputPath().exists()) { - testOutputsBuilder.add(Pair.of("test.xml", resolvedPaths.getXmlOutputPath())); + testOutputsBuilder.add( + Pair.of(TestFileNameConstants.TEST_LOG, action.getTestLog().getPath())); } + testOutputsBuilder.addAll(TestResult.testOutputsFromPaths(resolvedPaths)); actionExecutionContext .getEventBus() .post( @@ -202,21 +202,42 @@ public class StandaloneTestStrategy extends TestStrategy { // Rename outputs String namePrefix = FileSystemUtils.removeExtension(action.getTestLog().getExecPath().getBaseName()); - Path attemptsDir = - action.getTestLog().getPath().getParentDirectory().getChild(namePrefix + "_attempts"); + Path testRoot = action.getTestLog().getPath().getParentDirectory(); + Path attemptsDir = testRoot.getChild(namePrefix + "_attempts"); attemptsDir.createDirectory(); String attemptPrefix = "attempt_" + attempt; Path testLog = attemptsDir.getChild(attemptPrefix + ".log"); if (action.getTestLog().getPath().exists()) { action.getTestLog().getPath().renameTo(testLog); - testOutputsBuilder.add(Pair.of("test.log", testLog)); + testOutputsBuilder.add(Pair.of(TestFileNameConstants.TEST_LOG, testLog)); } + + // 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()); - if (resolvedPaths.getXmlOutputPath().exists()) { - Path destinationPath = attemptsDir.getChild(attemptPrefix + ".xml"); - resolvedPaths.getXmlOutputPath().renameTo(destinationPath); - testOutputsBuilder.add(Pair.of("test.xml", destinationPath)); + ImmutableList> testOutputs = TestResult.testOutputsFromPaths(resolvedPaths); + for (Pair testOutput : testOutputs) { + // e.g. /testRoot/test.dir/file, an example we follow throughout this loop's comments. + Path testOutputPath = testOutput.getSecond(); + + // 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. /attemptsDir/attempt_1.dir/file + Path destinationPath = attemptsDir.getRelative(destinationPathFragment); + destinationPath.getParentDirectory().createDirectory(); + + // Copy to the destination. + testOutputPath.renameTo(destinationPath); + + testOutputsBuilder.add(Pair.of(testOutput.getFirst(), destinationPath)); } + // Add the test log to the output dataBuilder.addFailedLogs(testLog.toString()); dataBuilder.addTestTimes(data.getTestTimes(0)); -- cgit v1.2.3