StandaloneTestStrategy sets the full list of outputs on the test spawn

All spawn strategies already treat all normal outputs as optional. Bazel checks
at the action level whether all action outputs are created, but does not check
at the spawn level. Spawn.getOptionalOutputs is therefore unnecessary, and
removed in this change.

The only place where this was set was in StandaloneTestStrategy, which now
specifies the full set of outputs, which is now computed by TestRunnerAction.
The internal test strategy implementations are also updated in this change.

While I'm at it, also remove the use of BaseSpawn and use SimpleSpawn instead.

This may go some way towards fixing #1413 and #942.

--
PiperOrigin-RevId: 149397100
MOS_MIGRATED_REVID=149397100

2 files changed, 16 insertions, 10 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 7e862d64ce..831183a059 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,13 +16,13 @@ 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;
-import com.google.devtools.build.lib.actions.BaseSpawn;
+import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ExecutionStrategy;
 import com.google.devtools.build.lib.actions.Executor;
+import com.google.devtools.build.lib.actions.SimpleSpawn;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
 import com.google.devtools.build.lib.actions.TestExecException;
@@ -112,15 +112,20 @@ public class StandaloneTestStrategy extends TestStrategy {
     info.putAll(action.getTestProperties().getExecutionInfo());
 
     Spawn spawn =
-        new BaseSpawn(
+        new SimpleSpawn(
+            action,
             getArgs(COLLECT_COVERAGE, action),
-            env,
-            info,
+            ImmutableMap.copyOf(env),
+            ImmutableMap.copyOf(info),
             new RunfilesSupplierImpl(
                 runfilesDir.asFragment(), action.getExecutionSettings().getRunfiles()),
-            action,
-            action.getTestProperties().getLocalResourceUsage(executionOptions.usingLocalTestJobs()),
-            ImmutableSet.of(resolvedPaths.getXmlOutputPath().relativeTo(execRoot)));
+            /*inputs=*/ImmutableList.copyOf(action.getInputs()),
+            /*tools=*/ImmutableList.<Artifact>of(),
+            /*filesetManifests=*/ImmutableList.<Artifact>of(),
+            ImmutableList.copyOf(action.getSpawnOutputs()),
+            action
+                .getTestProperties()
+                .getLocalResourceUsage(executionOptions.usingLocalTestJobs()));
 
     Executor executor = actionExecutionContext.getExecutor();
 
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index 1a31d5c232..9a99031dd8 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.exec;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.io.ByteStreams;
@@ -158,7 +159,7 @@ public abstract class TestStrategy implements TestActionContext {
    * @return the command line as string list.
    * @throws ExecException 
    */
-  protected List<String> getArgs(String coverageScript, TestRunnerAction testAction)
+  protected ImmutableList<String> getArgs(String coverageScript, TestRunnerAction testAction)
       throws ExecException {
     List<String> args = Lists.newArrayList();
     // TODO(ulfjack): This is incorrect for remote execution, where we need to consider the target
@@ -187,7 +188,7 @@ public abstract class TestStrategy implements TestActionContext {
     // Execute the test using the alias in the runfiles tree, as mandated by the Test Encyclopedia.
     args.add(execSettings.getExecutable().getRootRelativePath().getCallablePathString());
     args.addAll(execSettings.getArgs());
-    return args;
+    return ImmutableList.copyOf(args);
   }
 
   private static void addRunUnderArgs(TestRunnerAction testAction, List<String> args) {