commit 7967f3387e63723bb36b974f83a816265d404c38
author ruperts <ruperts@google.com> Tue Nov 21 16:37:13 2017 -0800
committer Copybara-Service <copybara-piper@google.com> Tue Nov 21 16:38:50 2017 -0800
tree 1533cc8bd2d63e1db9426d8795df902d104a3d1a
parent a2a3d8ca6634abb8f8dc184edb4ea01543de8505

Return SpawnResults using a List instead of a Set.

Currently we don't care about the list order of SpawnResults. However, we may care about the list order later. Also, if the equals() method for SpawnResults is ever changed then it may be problematic to return SpawnResults in a Set.

Aside: ActionResults use SpawnResults to calculate cumulative execution times for Actions, and may provide other metrics in future.

RELNOTES: None.
PiperOrigin-RevId: 176579460

src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java

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 bfa8cd8..41fcb3c 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
@@ -16,7 +16,6 @@
 
 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;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
@@ -51,8 +50,8 @@
 import com.google.devtools.build.lib.view.test.TestStatus.TestResultData.Builder;
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 /** Runs TestRunnerAction actions. */
 @ExecutionStrategy(
@@ -87,7 +86,7 @@
   }
 
   @Override
-  public Set<SpawnResult> exec(
+  public List<SpawnResult> exec(
       TestRunnerAction action, ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException {
     Path execRoot = actionExecutionContext.getExecRoot();
@@ -199,7 +198,7 @@
       finalizeTest(actionExecutionContext, action, dataBuilder.build());
 
       // TODO(b/62588075): Should we accumulate SpawnResults across test attempts instead of only
-      // returning the last set?
+      // returning the last list?
       return standaloneTestResult.spawnResults();
     } catch (IOException e) {
       actionExecutionContext.getEventHandler().handle(Event.error("Caught I/O exception: " + e));
@@ -351,7 +350,7 @@
     long startTime = actionExecutionContext.getClock().currentTimeMillis();
     SpawnActionContext spawnActionContext =
         actionExecutionContext.getSpawnActionContext(action.getMnemonic());
-    Set<SpawnResult> spawnResults = ImmutableSet.of();
+    List<SpawnResult> spawnResults = ImmutableList.of();
     try {
       try {
         if (executionOptions.testOutput.equals(TestOutputFormat.STREAMED)) {
@@ -374,7 +373,7 @@
             .setTestPassed(false)
             .setStatus(e.hasTimedOut() ? BlazeTestStatus.TIMEOUT : BlazeTestStatus.FAILED)
             .addFailedLogs(testLogPath.getPathString());
-        spawnResults = ImmutableSet.of(e.getSpawnResult());
+        spawnResults = ImmutableList.of(e.getSpawnResult());
       } finally {
         long duration = actionExecutionContext.getClock().currentTimeMillis() - startTime;
         builder.setStartTimeMillisEpoch(startTime);