Simplify SpawnRunner interface

It turns out that the SUCCESS status is often misunderstood to mean "zero exit",
even though this is clearly documented. I've decided to add another status for
non-zero exit, and use success only for zero exit to avoid this pitfall.

Also, many of the status codes are set, but never used. I decided to reduce the
number of status codes to only those that are actually relevant, which
simplifies further processing. Instead, we should add a string message for the
error case when we need one - we're not using it right now, so I decided not to
add that yet.

PiperOrigin-RevId: 177129441

2 files changed, 6 insertions, 4 deletions

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 e6c7ee9683..3547e31ac6 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
@@ -85,7 +85,8 @@ public class AbstractSpawnStrategyTest {
   public void testNonZeroExit() throws Exception {
     when(actionExecutionContext.getContext(eq(SpawnCache.class))).thenReturn(SpawnCache.NO_CACHE);
     when(actionExecutionContext.getExecRoot()).thenReturn(fs.getPath("/execroot"));
-    SpawnResult result = new SpawnResult.Builder().setStatus(Status.SUCCESS).setExitCode(1).build();
+    SpawnResult result =
+        new SpawnResult.Builder().setStatus(Status.NON_ZERO_EXIT).setExitCode(1).build();
     when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionPolicy.class)))
         .thenReturn(result);
 
@@ -151,7 +152,8 @@ 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.SUCCESS).setExitCode(1).build();
+    SpawnResult result =
+        new SpawnResult.Builder().setStatus(Status.NON_ZERO_EXIT).setExitCode(1).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/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
index c33c33cc2f..f7aa1d9ff3 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
@@ -343,7 +343,7 @@ public class LocalSpawnRunnerTest {
     assertThat(fs.getPath("/execroot").createDirectory()).isTrue();
     SpawnResult result = runner.exec(SIMPLE_SPAWN, policy);
     verify(factory).create(any(SubprocessBuilder.class));
-    assertThat(result.status()).isEqualTo(SpawnResult.Status.SUCCESS);
+    assertThat(result.status()).isEqualTo(SpawnResult.Status.NON_ZERO_EXIT);
     assertThat(result.exitCode()).isEqualTo(3);
     assertThat(result.setupSuccess()).isTrue();
     assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.getCachedShortHostName());
@@ -406,7 +406,7 @@ public class LocalSpawnRunnerTest {
     outErr = new FileOutErr();
     assertThat(fs.getPath("/execroot").createDirectory()).isTrue();
     SpawnResult reply = runner.exec(SIMPLE_SPAWN, policy);
-    assertThat(reply.status()).isEqualTo(SpawnResult.Status.LOCAL_ACTION_NOT_ALLOWED);
+    assertThat(reply.status()).isEqualTo(SpawnResult.Status.EXECUTION_DENIED);
     assertThat(reply.exitCode()).isEqualTo(-1);
     assertThat(reply.setupSuccess()).isFalse();
     assertThat(reply.getWallTime()).isEmpty();