Fix tests

bazelbuild · May 20, 2021 · 90f3ece · 90f3ece
1 parent 399747d
commit 90f3ece
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 3 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -172,6 +172,10 @@ RemoteCache getRemoteCache() {
     return cache;
   }
 
+  RemoteExecutionClient getRemoteExecutionClient() {
+    return executor;
+  }
+
   void setFilesToDownload(ImmutableSet<ActionInput> topLevelOutputs) {
     this.filesToDownload = Preconditions.checkNotNull(topLevelOutputs, "filesToDownload");
   }

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -218,6 +218,11 @@ public boolean mayBeCachedRemotely(Spawn spawn) {
     return remoteCache != null && Spawns.mayBeCached(spawn) && Spawns.mayBeCachedRemotely(spawn);
   }
 
+  /** Returns {@code true} if the result of spawn may be cached. */
+  public boolean mayBeCached(Spawn spawn) {
+    return remoteCache != null && Spawns.mayBeCached(spawn);
+  }
+
   /** Returns {@code true} if the spawn may be executed remotely. */
   public boolean mayBeExecutedRemotely(Spawn spawn) {
     return remoteCache != null && remoteExecutor != null && Spawns.mayBeExecutedRemotely(spawn);

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -805,7 +805,7 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
         Preconditions.checkNotNull(
             env.getOptions().getOptions(RemoteOptions.class), "RemoteOptions");
     RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode;
-    if (!remoteOutputsMode.downloadAllOutputs()) {
+    if (!remoteOutputsMode.downloadAllOutputs() && actionContextProvider.getRemoteCache() != null) {
       actionInputFetcher =
           new RemoteActionInputFetcher(
               env.getBuildRequestId(),

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
@@ -76,7 +76,9 @@ final class RemoteSpawnCache implements SpawnCache {
   @Override
   public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
       throws InterruptedException, IOException, ExecException {
-    if (!remoteExecutionService.mayBeCachedRemotely(spawn)) {
+    boolean mayBeCached = remoteExecutionService.mayBeCachedRemotely(spawn)
+        || (useDiskCache(options) && remoteExecutionService.mayBeCached(spawn));
+    if (!mayBeCached) {
       // returning SpawnCache.NO_RESULT_NO_STORE in case the caching is disabled or in case
       // the remote caching is disabled and the only configured cache is remote.
       return SpawnCache.NO_RESULT_NO_STORE;

diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java
@@ -15,6 +15,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -440,13 +441,52 @@ public void getCapabilities(
       remoteModule.beforeCommand(env);
 
       assertThat(Thread.interrupted()).isFalse();
-      assertThat(remoteModule.getActionContextProvider()).isNull();
+      RemoteActionContextProvider actionContextProvider = remoteModule.getActionContextProvider();
+      assertThat(actionContextProvider).isNotNull();
+      assertThat(actionContextProvider.getRemoteCache()).isNull();
+      assertThat(actionContextProvider.getRemoteExecutionClient()).isNull();
     } finally {
       cacheServer.shutdownNow();
       cacheServer.awaitTermination();
     }
   }
 
+  @Test
+  public void testLocalFallback_shouldIgnoreInaccessibleGrpcRemoteExecutor() throws Exception {
+    CapabilitiesImplBase executionServerCapabilitiesImpl = new CapabilitiesImplBase() {
+      @Override
+      public void getCapabilities(GetCapabilitiesRequest request, StreamObserver<ServerCapabilities> responseObserver) {
+        responseObserver.onError(new UnsupportedOperationException());
+      }
+    };
+    String executionServerName = "execution-server";
+    Server executionServer = createFakeServer(executionServerName, executionServerCapabilitiesImpl);
+    executionServer.start();
+
+    try {
+      RemoteModule remoteModule = new RemoteModule();
+      RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+      remoteOptions.remoteExecutor = executionServerName;
+      remoteOptions.remoteLocalFallback = true;
+      remoteModule.setChannelFactory(
+          (target, proxy, options, interceptors) ->
+              InProcessChannelBuilder.forName(target).directExecutor().build());
+
+      CommandEnvironment env = createTestCommandEnvironment(remoteOptions);
+
+      remoteModule.beforeCommand(env);
+
+      assertThat(Thread.interrupted()).isFalse();
+      RemoteActionContextProvider actionContextProvider = remoteModule.getActionContextProvider();
+      assertThat(actionContextProvider).isNotNull();
+      assertThat(actionContextProvider.getRemoteCache()).isNull();
+      assertThat(actionContextProvider.getRemoteExecutionClient()).isNull();
+    } finally {
+      executionServer.shutdownNow();
+      executionServer.awaitTermination();
+    }
+  }
+
   @Test
   public void testNetrc_emptyEnv_shouldIgnore() throws Exception {
     Map<String, String> clientEnv = ImmutableMap.of();