Fixing #4585: broken re-execution of orphaned actions.

This is an important regression, we will want to patch the fix into 0.10

TESTED=fixed unit test, with A/B testing
RELNOTES: Resolved an issue where a failure in the remote cache would not trigger local re-execution of an action.
PiperOrigin-RevId: 184991670

1 files changed, 76 insertions, 11 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
index c278581c2c..7167f2f48a 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
@@ -259,6 +259,7 @@ public class GrpcRemoteExecutionClientTest {
   @After
   public void tearDown() throws Exception {
     fakeServer.shutdownNow();
+    fakeServer.awaitTermination();
   }
 
   @Test
@@ -792,6 +793,66 @@ public class GrpcRemoteExecutionClientTest {
   }
 
   @Test
+  public void passRepeatedOrphanedCacheMissErrorWithStackTrace() throws Exception {
+    final Digest stdOutDigest = DIGEST_UTIL.computeAsUtf8("bloo");
+    final ActionResult actionResult =
+        ActionResult.newBuilder().setStdoutDigest(stdOutDigest).build();
+    serviceRegistry.addService(
+        new ActionCacheImplBase() {
+          @Override
+          public void getActionResult(
+              GetActionResultRequest request, StreamObserver<ActionResult> responseObserver) {
+            responseObserver.onNext(actionResult);
+            responseObserver.onCompleted();
+          }
+        });
+    serviceRegistry.addService(
+        new ExecutionImplBase() {
+          @Override
+          public void execute(ExecuteRequest request, StreamObserver<Operation> responseObserver) {
+            responseObserver.onNext(
+                Operation.newBuilder()
+                    .setDone(true)
+                    .setResponse(
+                        Any.pack(ExecuteResponse.newBuilder().setResult(actionResult).build()))
+                    .build());
+            responseObserver.onCompleted();
+          }
+        });
+    serviceRegistry.addService(
+        new ContentAddressableStorageImplBase() {
+          @Override
+          public void findMissingBlobs(
+              FindMissingBlobsRequest request,
+              StreamObserver<FindMissingBlobsResponse> responseObserver) {
+            responseObserver.onNext(FindMissingBlobsResponse.getDefaultInstance());
+            responseObserver.onCompleted();
+          }
+        });
+    serviceRegistry.addService(
+        new ByteStreamImplBase() {
+          @Override
+          public void read(ReadRequest request, StreamObserver<ReadResponse> responseObserver) {
+            assertThat(request.getResourceName().contains(stdOutDigest.getHash())).isTrue();
+            responseObserver.onError(Status.NOT_FOUND.asRuntimeException());
+          }
+        });
+
+    try {
+      client.exec(simpleSpawn, simplePolicy);
+      fail("Expected an exception");
+    } catch (SpawnExecException expected) {
+      assertThat(expected.getSpawnResult().status())
+          .isEqualTo(SpawnResult.Status.REMOTE_CACHE_FAILED);
+      assertThat(expected).hasMessageThat().contains(stdOutDigest.getHash());
+      // Ensure we also got back the stack trace.
+      assertThat(expected)
+          .hasMessageThat()
+          .contains("passRepeatedOrphanedCacheMissErrorWithStackTrace");
+    }
+  }
+
+  @Test
   public void remotelyReExecuteOrphanedCachedActions() throws Exception {
     final Digest stdOutDigest = DIGEST_UTIL.computeAsUtf8("stdout");
     final ActionResult actionResult =
@@ -807,10 +868,19 @@ public class GrpcRemoteExecutionClientTest {
         });
     serviceRegistry.addService(
         new ByteStreamImplBase() {
+          private boolean first = true;
+
           @Override
           public void read(ReadRequest request, StreamObserver<ReadResponse> responseObserver) {
-            // All reads are a cache miss.
-            responseObserver.onError(Status.NOT_FOUND.asRuntimeException());
+            // First read is a cache miss, next read succeeds.
+            if (first) {
+              first = false;
+              responseObserver.onError(Status.NOT_FOUND.asRuntimeException());
+            } else {
+              responseObserver.onNext(
+                  ReadResponse.newBuilder().setData(ByteString.copyFromUtf8("stdout")).build());
+              responseObserver.onCompleted();
+            }
           }
 
           @Override
@@ -858,14 +928,9 @@ public class GrpcRemoteExecutionClientTest {
           }
         });
 
-    try {
-      client.exec(simpleSpawn, simplePolicy);
-      fail("Expected an exception");
-    } catch (ExecException expected) {
-      assertThat(expected).hasMessageThat().contains("Missing digest");
-      assertThat(expected)
-          .hasMessageThat()
-          .contains(DIGEST_UTIL.computeAsUtf8("stdout").toString());
-    }
+    SpawnResult result = client.exec(simpleSpawn, simplePolicy);
+    assertThat(result.setupSuccess()).isTrue();
+    assertThat(result.exitCode()).isEqualTo(0);
+    assertThat(outErr.outAsLatin1()).isEqualTo("stdout");
   }
 }