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

2 files changed, 69 insertions, 41 deletions

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<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.
    */
@@ -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<Pair<String, Path>> 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<Pair<String, Path>> testOutputs = TestResult.testOutputsFromPaths(resolvedPaths);
+    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();
+
+      // 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));