Propagating remote results, including stdout/err, to Bazel on execution timeouts.

The refactoring to have an Exception that contains partial results will also be used in the next CL, in order to propagate and save remote server logs.

RELNOTES: None
PiperOrigin-RevId: 189344465

2 files changed, 36 insertions, 5 deletions

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 150e759718..98b56706d7 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
@@ -63,6 +63,7 @@ import com.google.devtools.common.options.Options;
 import com.google.devtools.remoteexecution.v1test.ActionResult;
 import com.google.devtools.remoteexecution.v1test.ExecuteRequest;
 import com.google.devtools.remoteexecution.v1test.ExecuteResponse;
+import com.google.rpc.Code;
 import java.io.IOException;
 import java.time.Duration;
 import java.util.Collection;
@@ -536,7 +537,18 @@ public class RemoteSpawnRunnerTest {
 
     ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build();
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
-    when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new TimeoutException());
+    ExecuteResponse resp =
+        ExecuteResponse.newBuilder()
+            .setResult(cachedResult)
+            .setStatus(
+                com.google.rpc.Status.newBuilder()
+                    .setCode(Code.DEADLINE_EXCEEDED.getNumber())
+                    .build())
+            .build();
+    when(executor.executeRemotely(any(ExecuteRequest.class)))
+        .thenThrow(
+            new Retrier.RetryException(
+                "", 1, new ExecutionStatusException(resp.getStatus(), resp)));
 
     Spawn spawn = newSimpleSpawn();
 
@@ -546,7 +558,7 @@ public class RemoteSpawnRunnerTest {
     assertThat(res.status()).isEqualTo(Status.TIMEOUT);
 
     verify(executor).executeRemotely(any(ExecuteRequest.class));
-    verify(cache, never()).download(eq(cachedResult), eq(execRoot), any(FileOutErr.class));
+    verify(cache).download(eq(cachedResult), eq(execRoot), any(FileOutErr.class));
   }
 
   @Test
@@ -572,7 +584,18 @@ public class RemoteSpawnRunnerTest {
 
     ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build();
     when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
-    when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new TimeoutException());
+    ExecuteResponse resp =
+        ExecuteResponse.newBuilder()
+            .setResult(cachedResult)
+            .setStatus(
+                com.google.rpc.Status.newBuilder()
+                    .setCode(Code.DEADLINE_EXCEEDED.getNumber())
+                    .build())
+            .build();
+    when(executor.executeRemotely(any(ExecuteRequest.class)))
+        .thenThrow(
+            new Retrier.RetryException(
+                "", 1, new ExecutionStatusException(resp.getStatus(), resp)));
 
     Spawn spawn = newSimpleSpawn();
 
@@ -582,7 +605,7 @@ public class RemoteSpawnRunnerTest {
     assertThat(res.status()).isEqualTo(Status.TIMEOUT);
 
     verify(executor).executeRemotely(any(ExecuteRequest.class));
-    verify(cache, never()).download(eq(cachedResult), eq(execRoot), any(FileOutErr.class));
+    verify(cache).download(eq(cachedResult), eq(execRoot), any(FileOutErr.class));
     verify(localRunner, never()).exec(eq(spawn), eq(policy));
   }
 
diff --git a/src/test/shell/bazel/remote_execution_test.sh b/src/test/shell/bazel/remote_execution_test.sh
index dff25048a1..2b800b3d54 100755
--- a/src/test/shell/bazel/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote_execution_test.sh
@@ -418,7 +418,11 @@ EOF
 
   cat > a/sleep.sh <<'EOF'
 #!/bin/sh
-sleep 2
+for i in {1..3}
+do
+    echo "Sleeping $i..."
+    sleep 1
+done
 EOF
   chmod +x a/sleep.sh
   bazel test \
@@ -429,6 +433,10 @@ EOF
       //a:sleep >& $TEST_log \
       && fail "Test failure (timeout) expected" || true
   expect_log "TIMEOUT"
+  expect_log "Sleeping 1..."
+  # The current implementation of the remote worker does not terminate actions
+  # when they time out, therefore we cannot verify that:
+  # expect_not_log "Sleeping 3..."
 }
 
 function test_passed_env_user() {