Ensure Runner name is always set.

RELNOTES: None.
PiperOrigin-RevId: 190617155

9 files changed, 85 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
index 10b484305a..e52f1cf623 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
@@ -360,6 +360,7 @@ public interface SpawnResult {
     private String failureMessage = "";
 
     public SpawnResult build() {
+      Preconditions.checkArgument(!runnerName.isEmpty());
       if (status == Status.SUCCESS) {
         Preconditions.checkArgument(exitCode == 0);
       }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index 9b515d16a1..a9ceb14e1e 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -129,6 +129,7 @@ final class RemoteSpawnCache implements SpawnCache {
                 .setStatus(Status.SUCCESS)
                 .setExitCode(result.getExitCode())
                 .setCacheHit(true)
+                .setRunnerName("remote cache hit")
                 .build();
         return SpawnCache.success(spawnResult);
       }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index bb858a924a..2700ba147e 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -169,6 +169,7 @@ class RemoteSpawnRunner implements SpawnRunner {
           try {
             return downloadRemoteResults(cachedResult, policy.getFileOutErr())
                 .setCacheHit(true)
+                .setRunnerName("remote cache hit")
                 .build();
           } catch (CacheNotFoundException e) {
             // No cache hit, so we fall through to local or remote execution.
@@ -210,7 +211,7 @@ class RemoteSpawnRunner implements SpawnRunner {
 
       try {
         return downloadRemoteResults(result, policy.getFileOutErr())
-            .setRunnerName(remoteCacheHit ? "" : getName())
+            .setRunnerName(remoteCacheHit ? "remote cache hit" : getName())
             .setCacheHit(remoteCacheHit)
             .build();
       } catch (IOException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionResultTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionResultTest.java
index 207df55f47..820261ab0f 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ActionResultTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ActionResultTest.java
@@ -50,6 +50,7 @@ public final class ActionResultTest {
             .setNumBlockInputOperations(20)
             .setNumInvoluntaryContextSwitches(30)
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     List<SpawnResult> spawnResults = ImmutableList.of(spawnResult);
     ActionResult actionResult = ActionResult.create(spawnResults);
@@ -73,6 +74,7 @@ public final class ActionResultTest {
             .setNumBlockInputOperations(20)
             .setNumInvoluntaryContextSwitches(30)
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     SpawnResult spawnResult2 =
         new SpawnResult.Builder()
@@ -83,6 +85,7 @@ public final class ActionResultTest {
             .setNumBlockInputOperations(200)
             .setNumInvoluntaryContextSwitches(300)
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     SpawnResult spawnResult3 =
         new SpawnResult.Builder()
@@ -93,6 +96,7 @@ public final class ActionResultTest {
             .setNumBlockInputOperations(2000)
             .setNumInvoluntaryContextSwitches(3000)
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     List<SpawnResult> spawnResults = ImmutableList.of(spawnResult1, spawnResult2, spawnResult3);
     ActionResult actionResult = ActionResult.create(spawnResults);
@@ -108,11 +112,20 @@ public final class ActionResultTest {
   @Test
   public void testCumulativeCommandExecutionTime_ManyEmptySpawnResults() {
     SpawnResult spawnResult1 =
-        new SpawnResult.Builder().setStatus(SpawnResult.Status.SUCCESS).build();
+        new SpawnResult.Builder()
+            .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
+            .build();
     SpawnResult spawnResult2 =
-        new SpawnResult.Builder().setStatus(SpawnResult.Status.SUCCESS).build();
+        new SpawnResult.Builder()
+            .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
+            .build();
     SpawnResult spawnResult3 =
-        new SpawnResult.Builder().setStatus(SpawnResult.Status.SUCCESS).build();
+        new SpawnResult.Builder()
+            .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
+            .build();
     List<SpawnResult> spawnResults = ImmutableList.of(spawnResult1, spawnResult2, spawnResult3);
     ActionResult actionResult = ActionResult.create(spawnResults);
     assertThat(actionResult.cumulativeCommandExecutionWallTime()).isEmpty();
@@ -130,16 +143,19 @@ public final class ActionResultTest {
         new SpawnResult.Builder()
             .setUserTime(Duration.ofMillis(2))
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     SpawnResult spawnResult2 =
         new SpawnResult.Builder()
             .setUserTime(Duration.ofMillis(3))
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     SpawnResult spawnResult3 =
         new SpawnResult.Builder()
             .setUserTime(Duration.ofMillis(4))
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     List<SpawnResult> spawnResults = ImmutableList.of(spawnResult1, spawnResult2, spawnResult3);
     ActionResult actionResult = ActionResult.create(spawnResults);
@@ -153,16 +169,19 @@ public final class ActionResultTest {
         new SpawnResult.Builder()
             .setSystemTime(Duration.ofMillis(33))
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     SpawnResult spawnResult2 =
         new SpawnResult.Builder()
             .setSystemTime(Duration.ofMillis(7))
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     SpawnResult spawnResult3 =
         new SpawnResult.Builder()
             .setSystemTime(Duration.ofMillis(2))
             .setStatus(SpawnResult.Status.SUCCESS)
+            .setRunnerName("test")
             .build();
     List<SpawnResult> spawnResults = ImmutableList.of(spawnResult1, spawnResult2, spawnResult3);
     ActionResult actionResult = ActionResult.create(spawnResults);
diff --git a/src/test/java/com/google/devtools/build/lib/actions/SpawnResultTest.java b/src/test/java/com/google/devtools/build/lib/actions/SpawnResultTest.java
index 4fedd29cb1..6dccb47995 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/SpawnResultTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/SpawnResultTest.java
@@ -33,6 +33,7 @@ public final class SpawnResultTest {
             .setStatus(SpawnResult.Status.TIMEOUT)
             .setWallTime(Duration.ofSeconds(5))
             .setExitCode(1)
+            .setRunnerName("test")
             .build();
     assertThat(r.getDetailMessage("", "", false, false))
         .contains("(failed due to timeout after 5.00 seconds.)");
@@ -41,7 +42,11 @@ public final class SpawnResultTest {
   @Test
   public void getTimeoutMessageNoTime() {
     SpawnResult r =
-        new SpawnResult.Builder().setStatus(SpawnResult.Status.TIMEOUT).setExitCode(1).build();
+        new SpawnResult.Builder()
+            .setStatus(SpawnResult.Status.TIMEOUT)
+            .setExitCode(1)
+            .setRunnerName("test")
+            .build();
     assertThat(r.getDetailMessage("", "", false, false))
         .contains("(failed due to timeout.)");
   }
diff --git a/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java
index c4d3b5bdbb..42de712e40 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java
@@ -69,7 +69,8 @@ public class AbstractSpawnStrategyTest {
   public void testZeroExit() throws Exception {
     when(actionExecutionContext.getContext(eq(SpawnCache.class))).thenReturn(SpawnCache.NO_CACHE);
     when(actionExecutionContext.getExecRoot()).thenReturn(fs.getPath("/execroot"));
-    SpawnResult spawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).build();
+    SpawnResult spawnResult =
+        new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
     when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class)))
         .thenReturn(spawnResult);
 
@@ -87,7 +88,11 @@ public class AbstractSpawnStrategyTest {
     when(actionExecutionContext.getContext(eq(SpawnCache.class))).thenReturn(SpawnCache.NO_CACHE);
     when(actionExecutionContext.getExecRoot()).thenReturn(execRoot);
     SpawnResult result =
-        new SpawnResult.Builder().setStatus(Status.NON_ZERO_EXIT).setExitCode(1).build();
+        new SpawnResult.Builder()
+            .setStatus(Status.NON_ZERO_EXIT)
+            .setExitCode(1)
+            .setRunnerName("test")
+            .build();
     when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class)))
         .thenReturn(result);
 
@@ -105,7 +110,8 @@ public class AbstractSpawnStrategyTest {
   @Test
   public void testCacheHit() throws Exception {
     SpawnCache cache = mock(SpawnCache.class);
-    SpawnResult spawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).build();
+    SpawnResult spawnResult =
+        new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
     when(cache.lookup(any(Spawn.class), any(SpawnExecutionPolicy.class)))
         .thenReturn(SpawnCache.success(spawnResult));
     when(actionExecutionContext.getContext(eq(SpawnCache.class))).thenReturn(cache);
@@ -128,7 +134,8 @@ public class AbstractSpawnStrategyTest {
 
     when(actionExecutionContext.getContext(eq(SpawnCache.class))).thenReturn(cache);
     when(actionExecutionContext.getExecRoot()).thenReturn(fs.getPath("/execroot"));
-    SpawnResult spawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).build();
+    SpawnResult spawnResult =
+        new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
     when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class)))
         .thenReturn(spawnResult);
 
@@ -154,7 +161,11 @@ public class AbstractSpawnStrategyTest {
     when(actionExecutionContext.getContext(eq(SpawnCache.class))).thenReturn(cache);
     when(actionExecutionContext.getExecRoot()).thenReturn(fs.getPath("/execroot"));
     SpawnResult result =
-        new SpawnResult.Builder().setStatus(Status.NON_ZERO_EXIT).setExitCode(1).build();
+        new SpawnResult.Builder()
+            .setStatus(Status.NON_ZERO_EXIT)
+            .setExitCode(1)
+            .setRunnerName("test")
+            .build();
     when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class))).thenReturn(result);
 
     try {
diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
index 57ad24332d..34058a42f5 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
@@ -125,6 +125,7 @@ public final class StandaloneTestStrategyTest extends BuildViewTestCase {
         new SpawnResult.Builder()
             .setStatus(Status.SUCCESS)
             .setWallTime(Duration.ofMillis(10))
+            .setRunnerName("test")
             .build();
     when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult));
 
@@ -200,7 +201,11 @@ public final class StandaloneTestStrategyTest extends BuildViewTestCase {
     when(actionExecutionContext.getFileOutErr()).thenReturn(outErr);
 
     SpawnResult expectedSpawnResult =
-        new SpawnResult.Builder().setStatus(Status.NON_ZERO_EXIT).setExitCode(1).build();
+        new SpawnResult.Builder()
+            .setStatus(Status.NON_ZERO_EXIT)
+            .setExitCode(1)
+            .setRunnerName("test")
+            .build();
     when(spawnActionContext.exec(any(), any()))
         .thenThrow(
             new SpawnExecException(
@@ -283,7 +288,8 @@ public final class StandaloneTestStrategyTest extends BuildViewTestCase {
     FileOutErr outErr = new FileOutErr(outPath, errPath);
     when(actionExecutionContext.getFileOutErr()).thenReturn(outErr);
 
-    SpawnResult expectedSpawnResult = new SpawnResult.Builder().setStatus(Status.SUCCESS).build();
+    SpawnResult expectedSpawnResult =
+        new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
     when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult));
     when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext);
 
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
index 9faf8e4d5c..a41f9c4f96 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
@@ -248,7 +248,12 @@ public class RemoteSpawnCacheTest {
   public void cacheMiss() throws Exception {
     CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy);
     assertThat(entry.hasResult()).isFalse();
-    SpawnResult result = new SpawnResult.Builder().setExitCode(0).setStatus(Status.SUCCESS).build();
+    SpawnResult result =
+        new SpawnResult.Builder()
+            .setExitCode(0)
+            .setStatus(Status.SUCCESS)
+            .setRunnerName("test")
+            .build();
     ImmutableList<Path> outputFiles = ImmutableList.of(fs.getPath("/random/file"));
     Mockito.doAnswer(
             new Answer<Void>() {
@@ -285,7 +290,12 @@ public class RemoteSpawnCacheTest {
     verify(remoteCache, never())
         .getCachedActionResult(any(ActionKey.class));
     assertThat(entry.hasResult()).isFalse();
-    SpawnResult result = new SpawnResult.Builder().setExitCode(0).setStatus(Status.SUCCESS).build();
+    SpawnResult result =
+        new SpawnResult.Builder()
+            .setExitCode(0)
+            .setStatus(Status.SUCCESS)
+            .setRunnerName("test")
+            .build();
     entry.store(result);
     ImmutableList<Path> outputFiles = ImmutableList.of(fs.getPath("/random/file"));
     verify(remoteCache)
@@ -300,7 +310,11 @@ public class RemoteSpawnCacheTest {
     verify(remoteCache).getCachedActionResult(any(ActionKey.class));
     assertThat(entry.hasResult()).isFalse();
     SpawnResult result =
-        new SpawnResult.Builder().setExitCode(1).setStatus(Status.NON_ZERO_EXIT).build();
+        new SpawnResult.Builder()
+            .setExitCode(1)
+            .setStatus(Status.NON_ZERO_EXIT)
+            .setRunnerName("test")
+            .build();
     ImmutableList<Path> outputFiles = ImmutableList.of(fs.getPath("/random/file"));
     entry.store(result);
     verify(remoteCache)
@@ -311,7 +325,12 @@ public class RemoteSpawnCacheTest {
   public void printWarningIfUploadFails() throws Exception {
     CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy);
     assertThat(entry.hasResult()).isFalse();
-    SpawnResult result = new SpawnResult.Builder().setExitCode(0).setStatus(Status.SUCCESS).build();
+    SpawnResult result =
+        new SpawnResult.Builder()
+            .setExitCode(0)
+            .setStatus(Status.SUCCESS)
+            .setRunnerName("test")
+            .build();
     ImmutableList<Path> outputFiles = ImmutableList.of(fs.getPath("/random/file"));
 
     doThrow(new IOException("cache down"))
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index 29470f4438..e16f8e4874 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -359,7 +359,12 @@ public class RemoteSpawnRunnerTest {
             any(FileOutErr.class),
             eq(true));
 
-    SpawnResult res = new SpawnResult.Builder().setStatus(Status.SUCCESS).setExitCode(0).build();
+    SpawnResult res =
+        new SpawnResult.Builder()
+            .setStatus(Status.SUCCESS)
+            .setExitCode(0)
+            .setRunnerName("test")
+            .build();
     when(localRunner.exec(eq(spawn), eq(policy))).thenReturn(res);
 
     assertThat(runner.exec(spawn, policy)).isEqualTo(res);