BEP: Provide more useful data in TestResult events

For individual test actions, more useful data can be provided
than just the success status. Do so.

Also, to keep each log reported only once, send a new
TestAttempt event for the final test attempt, but don't
have the TestResult (which contains to union of all test
files) an instance of the BuildEvent.

As we now report test results in a separate object, drop
the test data and directly pass in the files created. In
this way, we also get all the moved files reported correctly
in the individual attempts.

--
Change-Id: Ic04b7bad4f92a381bd4d1b4ec91f743b89f81f84
Reviewed-on: https://cr.bazel.build/8694
PiperOrigin-RevId: 147149025
MOS_MIGRATED_REVID=147149025

1 files changed, 23 insertions, 7 deletions

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 5e33a9ba77..c0739034e7 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
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.exec;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -38,6 +39,7 @@ import com.google.devtools.build.lib.rules.test.TestAttempt;
 import com.google.devtools.build.lib.rules.test.TestResult;
 import com.google.devtools.build.lib.rules.test.TestRunnerAction;
 import com.google.devtools.build.lib.rules.test.TestRunnerAction.ResolvedPaths;
+import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -155,7 +157,19 @@ public class StandaloneTestStrategy extends TestStrategy {
                 workingDirectory);
       }
       processLastTestAttempt(attempt, dataBuilder, data);
-      finalizeTest(attempt, actionExecutionContext, action, dataBuilder.build());
+      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()));
+      }
+      executor
+          .getEventBus()
+          .post(
+              new TestAttempt(
+                  action, attempt, data.getTestPassed(), testOutputsBuilder.build(), true));
+      finalizeTest(actionExecutionContext, action, dataBuilder.build());
     } catch (IOException e) {
       executor.getEventHandler().handle(Event.error("Caught I/O exception: " + e));
       throw new EnvironmentalExecException("unexpected I/O exception", e);
@@ -170,6 +184,7 @@ public class StandaloneTestStrategy extends TestStrategy {
       TestResultData data,
       FileOutErr outErr)
       throws IOException {
+    ImmutableList.Builder<Pair<String, Path>> testOutputsBuilder = new ImmutableList.Builder<>();
     // Rename outputs
     String namePrefix =
         FileSystemUtils.removeExtension(action.getTestLog().getExecPath().getBaseName());
@@ -180,17 +195,21 @@ public class StandaloneTestStrategy extends TestStrategy {
     Path testLog = attemptsDir.getChild(attemptPrefix + ".log");
     if (action.getTestLog().getPath().exists()) {
       action.getTestLog().getPath().renameTo(testLog);
+      testOutputsBuilder.add(Pair.of("test.log", testLog));
     }
     ResolvedPaths resolvedPaths = action.resolve(executor.getExecRoot());
     if (resolvedPaths.getXmlOutputPath().exists()) {
       Path destinationPath = attemptsDir.getChild(attemptPrefix + ".xml");
       resolvedPaths.getXmlOutputPath().renameTo(destinationPath);
+      testOutputsBuilder.add(Pair.of("test.xml", destinationPath));
     }
     // Add the test log to the output
     dataBuilder.addFailedLogs(testLog.toString());
     dataBuilder.addTestTimes(data.getTestTimes(0));
     dataBuilder.addAllTestProcessTimes(data.getTestProcessTimesList());
-    executor.getEventBus().post(new TestAttempt(action, attempt));
+    executor
+        .getEventBus()
+        .post(new TestAttempt(action, attempt, data.getTestPassed(), testOutputsBuilder.build()));
     processTestOutput(executor, outErr, new TestResult(action, data, false), testLog);
   }
 
@@ -365,12 +384,9 @@ public class StandaloneTestStrategy extends TestStrategy {
   }
 
   private final void finalizeTest(
-      int attempt,
-      ActionExecutionContext actionExecutionContext,
-      TestRunnerAction action,
-      TestResultData data)
+      ActionExecutionContext actionExecutionContext, TestRunnerAction action, TestResultData data)
       throws IOException, ExecException {
-    TestResult result = new TestResult(action, data, false, attempt);
+    TestResult result = new TestResult(action, data, false);
     postTestResult(actionExecutionContext.getExecutor(), result);
 
     processTestOutput(