From 4dd6f00db129c2082cedde84e20d23e852a0a2e6 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 27 Mar 2018 08:15:39 -0700 Subject: Ensure Runner name is always set. RELNOTES: None. PiperOrigin-RevId: 190617155 --- .../devtools/build/lib/actions/SpawnResult.java | 1 + .../build/lib/remote/RemoteSpawnCache.java | 1 + .../build/lib/remote/RemoteSpawnRunner.java | 3 ++- .../build/lib/actions/ActionResultTest.java | 25 +++++++++++++++++--- .../build/lib/actions/SpawnResultTest.java | 7 +++++- .../build/lib/exec/AbstractSpawnStrategyTest.java | 21 +++++++++++++---- .../build/lib/exec/StandaloneTestStrategyTest.java | 10 ++++++-- .../build/lib/remote/RemoteSpawnCacheTest.java | 27 ++++++++++++++++++---- .../build/lib/remote/RemoteSpawnRunnerTest.java | 7 +++++- 9 files changed, 85 insertions(+), 17 deletions(-) (limited to 'src') 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 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 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 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 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 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 outputFiles = ImmutableList.of(fs.getPath("/random/file")); Mockito.doAnswer( new Answer() { @@ -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 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 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 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); -- cgit v1.2.3