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

4 files changed, 1 insertions, 56 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
index e3143fddcc..901b225afe 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.actions;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
@@ -31,7 +30,6 @@ public class BaseSpawn implements Spawn {
   private final ImmutableList<String> arguments;
   private final ImmutableMap<String, String> environment;
   private final ImmutableMap<String, String> executionInfo;
-  private final ImmutableSet<PathFragment> optionalOutputFiles;
   private final RunfilesSupplier runfilesSupplier;
   private final ActionExecutionMetadata action;
   private final ResourceSet localResources;
@@ -42,39 +40,15 @@ public class BaseSpawn implements Spawn {
       Map<String, String> executionInfo,
       RunfilesSupplier runfilesSupplier,
       ActionExecutionMetadata action,
-      ResourceSet localResources,
-      Collection<PathFragment> optionalOutputFiles) {
+      ResourceSet localResources) {
     this.arguments = ImmutableList.copyOf(arguments);
     this.environment = ImmutableMap.copyOf(environment);
     this.executionInfo = ImmutableMap.copyOf(executionInfo);
     this.runfilesSupplier = runfilesSupplier;
     this.action = action;
     this.localResources = localResources;
-    this.optionalOutputFiles = ImmutableSet.copyOf(optionalOutputFiles);
-  }
-
-  /**
-   * Returns a new Spawn. The caller must not modify the parameters after the call; neither will
-   * this method.
-   */
-  public BaseSpawn(
-      List<String> arguments,
-      Map<String, String> environment,
-      Map<String, String> executionInfo,
-      RunfilesSupplier runfilesSupplier,
-      ActionExecutionMetadata action,
-      ResourceSet localResources) {
-    this(
-        arguments,
-        environment,
-        executionInfo,
-        runfilesSupplier,
-        action,
-        localResources,
-        ImmutableSet.<PathFragment>of());
   }
 
-  /** Returns a new Spawn. */
   public BaseSpawn(
       List<String> arguments,
       Map<String, String> environment,
@@ -170,11 +144,6 @@ public class BaseSpawn implements Spawn {
   }
 
   @Override
-  public Collection<PathFragment> getOptionalOutputFiles() {
-    return optionalOutputFiles;
-  }
-
-  @Override
   public ActionExecutionMetadata getResourceOwner() {
     return action;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
index 503979c2a2..8341301053 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.actions;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 
 /**
@@ -82,11 +81,6 @@ public class DelegateSpawn implements Spawn {
   }
 
   @Override
-  public Collection<PathFragment> getOptionalOutputFiles() {
-    return spawn.getOptionalOutputFiles();
-  }
-
-  @Override
   public ActionExecutionMetadata getResourceOwner() {
     return spawn.getResourceOwner();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java
index d6fc5a3a40..564460e726 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java
@@ -17,8 +17,6 @@ package com.google.devtools.build.lib.actions;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.vfs.PathFragment;
 import javax.annotation.concurrent.Immutable;
 
 /**
@@ -36,7 +34,6 @@ public final class SimpleSpawn implements Spawn {
   private final RunfilesSupplier runfilesSupplier;
   private final ImmutableList<Artifact> filesetManifests;
   private final ImmutableList<? extends ActionInput> outputs;
-  private final ImmutableSet<PathFragment> optionalOutputFiles;
   private final ResourceSet localResources;
 
   public SimpleSpawn(
@@ -49,7 +46,6 @@ public final class SimpleSpawn implements Spawn {
       ImmutableList<? extends ActionInput> tools,
       ImmutableList<Artifact> filesetManifests,
       ImmutableList<? extends ActionInput> outputs,
-      ImmutableSet<PathFragment> optionalOutputFiles,
       ResourceSet localResources) {
     this.owner = Preconditions.checkNotNull(owner);
     this.arguments = Preconditions.checkNotNull(arguments);
@@ -61,7 +57,6 @@ public final class SimpleSpawn implements Spawn {
         runfilesSupplier == null ? EmptyRunfilesSupplier.INSTANCE : runfilesSupplier;
     this.filesetManifests = Preconditions.checkNotNull(filesetManifests);
     this.outputs = Preconditions.checkNotNull(outputs);
-    this.optionalOutputFiles = Preconditions.checkNotNull(optionalOutputFiles);
     this.localResources = Preconditions.checkNotNull(localResources);
   }
 
@@ -83,7 +78,6 @@ public final class SimpleSpawn implements Spawn {
         ImmutableList.<Artifact>of(),
         ImmutableList.<Artifact>of(),
         outputs,
-        ImmutableSet.<PathFragment>of(),
         localResources);
   }
 
@@ -138,11 +132,6 @@ public final class SimpleSpawn implements Spawn {
   }
 
   @Override
-  public ImmutableSet<PathFragment> getOptionalOutputFiles() {
-    return optionalOutputFiles;
-  }
-
-  @Override
   public ActionExecutionMetadata getResourceOwner() {
     return owner;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
index 26f636b806..01e901042b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.actions;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 
 /**
@@ -106,12 +105,6 @@ public interface Spawn {
   Collection<? extends ActionInput> getOutputFiles();
 
   /**
-   * Instructs the spawn strategy to try to fetch these optional output files in addition to the
-   * usual output artifacts. The PathFragments should be relative to the exec root.
-   */
-  Collection<PathFragment> getOptionalOutputFiles();
-
-  /**
    * Returns the resource owner for local fallback.
    */
   ActionExecutionMetadata getResourceOwner();