diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 26d92e0f6f370c..bf277a0649fb78 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -108,4 +108,5 @@ Jonathan Dierksen Tony Aiuto Andy Scott Jamie Snape -Irina Chernushina \ No newline at end of file +Irina Chernushina +C. Sean Young diff --git a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java index 40ba2a50ab7b74..0f9837b47ae87e 100644 --- a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java @@ -36,14 +36,17 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.SpawnStrategy; +import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.exec.ExecutionPolicy; +import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.DynamicExecution; import com.google.devtools.build.lib.server.FailureDetails.DynamicExecution.Code; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; +import java.util.List; import java.util.Optional; import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; @@ -276,11 +279,13 @@ static ImmutableList waitBranches( String.format( "Neither branch of %s cancelled the other one.", spawn.getResourceOwner().getPrimaryOutput().prettyPrint())); - } else if (remoteResult != null) { - return remoteResult; } else if (localResult != null) { return localResult; + } else if (remoteResult != null) { + return remoteResult; } else { + // TODO(b/173153395): Sometimes gets thrown for currently unknown reasons. + // (sometimes happens in relation to the whole dynamic execution being cancelled) throw new AssertionError( String.format( "Neither branch of %s completed. Local was %scancelled and remote was %scancelled", @@ -331,18 +336,142 @@ static void verifyAvailabilityInfo(DynamicExecutionOptions options, Spawn spawn) } } + private static boolean canExecLocalSpawn( + Spawn spawn, + ExecutionPolicy executionPolicy, + ActionContext.ActionContextRegistry actionContextRegistry, + DynamicStrategyRegistry dynamicStrategyRegistry) { + if (!executionPolicy.canRunLocally()) { + return false; + } + List localStrategies = + dynamicStrategyRegistry.getDynamicSpawnActionContexts( + spawn, DynamicStrategyRegistry.DynamicMode.LOCAL); + return localStrategies.stream() + .anyMatch( + s -> + (s.canExec(spawn, actionContextRegistry) + || s.canExecWithLegacyFallback(spawn, actionContextRegistry))); + } + + private boolean canExecLocal( + Spawn spawn, + ExecutionPolicy mainSpawnExecutionPolicy, + ActionContext.ActionContextRegistry actionContextRegistry, + DynamicStrategyRegistry dynamicStrategyRegistry) { + if (!canExecLocalSpawn( + spawn, mainSpawnExecutionPolicy, actionContextRegistry, dynamicStrategyRegistry)) { + return false; + } + // Present if there is a extra local spawn. Unset if not. + Optional canLocalSpawn = + getExtraSpawnForLocalExecution + .apply(spawn) + .map( + extraSpawn -> + canExecLocalSpawn( + extraSpawn, + getExecutionPolicy.apply(extraSpawn), + actionContextRegistry, + dynamicStrategyRegistry)); + return canLocalSpawn.orElse(true); + } + + private static boolean canExecRemote( + Spawn spawn, + ExecutionPolicy executionPolicy, + ActionContext.ActionContextRegistry actionContextRegistry, + DynamicStrategyRegistry dynamicStrategyRegistry) { + if (!executionPolicy.canRunRemotely()) { + return false; + } + List remoteStrategies = + dynamicStrategyRegistry.getDynamicSpawnActionContexts( + spawn, DynamicStrategyRegistry.DynamicMode.REMOTE); + return remoteStrategies.stream().anyMatch(s -> s.canExec(spawn, actionContextRegistry)); + } + + @Override + public boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry) { + ExecutionPolicy executionPolicy = getExecutionPolicy.apply(spawn); + DynamicStrategyRegistry dynamicStrategyRegistry = + actionContextRegistry.getContext(DynamicStrategyRegistry.class); + + return canExecLocal(spawn, executionPolicy, actionContextRegistry, dynamicStrategyRegistry) + || canExecRemote(spawn, executionPolicy, actionContextRegistry, dynamicStrategyRegistry); + } + + /** + * Returns an error string for being unable to execute locally and/or remotely the given execution + * state. + * + *

Usage note, this method is only to be called after an impossible condition is already + * detected by the caller, as all this does is give an error string to put in the exception. + * + * @param spawn The action that needs to be executed + * @param localAllowedBySpawnExecutionPolicy whether the execution policy for this spawn allows + * trying local execution. + * @param remoteAllowedBySpawnExecutionPolicy whether the execution policy for this spawn allows + * trying remote execution. + */ + private static String getNoCanExecFailureMessage( + Spawn spawn, + boolean localAllowedBySpawnExecutionPolicy, + boolean remoteAllowedBySpawnExecutionPolicy) { + // TODO(b/188387840): Can't use Spawn.toString() here because tests report FakeOwner instances + // as the resource owner, and those cause toStrings to throw if no primary output. + // TODO(b/188402092): Even if the above is fixed, we still don't want to use Spawn.toString() + // until the mnemonic is included in the output unconditionally. Too useful for the error + // message. + if (!localAllowedBySpawnExecutionPolicy && !remoteAllowedBySpawnExecutionPolicy) { + return "Neither local nor remote execution allowed for action " + spawn.getMnemonic(); + } else if (!remoteAllowedBySpawnExecutionPolicy) { + return "No usable dynamic_local_strategy found (and remote execution disabled) for action " + + spawn.getMnemonic(); + } else if (!localAllowedBySpawnExecutionPolicy) { + return "No usable dynamic_remote_strategy found (and local execution disabled) for action " + + spawn.getMnemonic(); + } else { + return "No usable dynamic_local_strategy or dynamic_remote_strategy found for action " + + spawn.getMnemonic(); + } + } + @Override public ImmutableList exec( final Spawn spawn, final ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException { DynamicSpawnStrategy.verifyAvailabilityInfo(options, spawn); ExecutionPolicy executionPolicy = getExecutionPolicy.apply(spawn); - if (executionPolicy.canRunLocallyOnly()) { - return runLocally(spawn, actionExecutionContext, null); - } - if (executionPolicy.canRunRemotelyOnly()) { + + DynamicStrategyRegistry dynamicStrategyRegistry = + actionExecutionContext.getContext(DynamicStrategyRegistry.class); + boolean localCanExec = + canExecLocal(spawn, executionPolicy, actionExecutionContext, dynamicStrategyRegistry); + + boolean remoteCanExec = + canExecRemote(spawn, executionPolicy, actionExecutionContext, dynamicStrategyRegistry); + + if (!localCanExec && !remoteCanExec) { + FailureDetail failure = + FailureDetail.newBuilder() + .setMessage( + getNoCanExecFailureMessage( + spawn, executionPolicy.canRunLocally(), executionPolicy.canRunRemotely())) + .setDynamicExecution( + DynamicExecution.newBuilder().setCode(Code.NO_USABLE_STRATEGY_FOUND).build()) + .setSpawn( + FailureDetails.Spawn.newBuilder() + .setCode(FailureDetails.Spawn.Code.NO_USABLE_STRATEGY_FOUND) + .build()) + .build(); + throw new UserExecException(failure); + } else if (!localCanExec && remoteCanExec) { return runRemotely(spawn, actionExecutionContext, null); + } else if (localCanExec && !remoteCanExec) { + return runLocally(spawn, actionExecutionContext, null); } + // else both can exec. Fallthrough to below. // Semaphores to track termination of each branch. These are necessary to wait for the branch to // finish its own cleanup (e.g. terminating subprocesses) once it has been cancelled. @@ -458,28 +587,6 @@ public ImmutableList callImpl(ActionExecutionContext context) } } - @Override - public boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry) { - DynamicStrategyRegistry dynamicStrategyRegistry = - actionContextRegistry.getContext(DynamicStrategyRegistry.class); - for (SandboxedSpawnStrategy strategy : - dynamicStrategyRegistry.getDynamicSpawnActionContexts( - spawn, DynamicStrategyRegistry.DynamicMode.LOCAL)) { - if (strategy.canExec(spawn, actionContextRegistry) - || strategy.canExecWithLegacyFallback(spawn, actionContextRegistry)) { - return true; - } - } - for (SandboxedSpawnStrategy strategy : - dynamicStrategyRegistry.getDynamicSpawnActionContexts( - spawn, DynamicStrategyRegistry.DynamicMode.REMOTE)) { - if (strategy.canExec(spawn, actionContextRegistry)) { - return true; - } - } - return false; - } - @Override public void usedContext(ActionContext.ActionContextRegistry actionContextRegistry) { actionContextRegistry @@ -496,6 +603,12 @@ private static FileOutErr getSuffixedFileOutErr(FileOutErr fileOutErr, String su outDir.getChild(outBaseName + suffix), errDir.getChild(errBaseName + suffix)); } + /** + * Try to run the given spawn locally. + * + *

Precondition: At least one {@code dynamic_local_strategy} returns {@code true} from its + * {@link SpawnStrategy#canExec canExec} method for the given {@code spawn}. + */ private ImmutableList runLocally( Spawn spawn, ActionExecutionContext actionExecutionContext, @@ -539,12 +652,15 @@ private static ImmutableList runSpawnLocally( return strategy.exec(spawn, actionExecutionContext, stopConcurrentSpawns); } } - throw new RuntimeException( - String.format( - "executorCreated not yet called or no default dynamic_local_strategy set for %s", - spawn.getMnemonic())); + throw new AssertionError("canExec passed but no usable local strategy for action " + spawn); } + /** + * Try to run the given spawn locally. + * + *

Precondition: At least one {@code dynamic_remote_strategy} returns {@code true} from its + * {@link SpawnStrategy#canExec canExec} method for the given {@code spawn}. + */ private static ImmutableList runRemotely( Spawn spawn, ActionExecutionContext actionExecutionContext, @@ -571,10 +687,7 @@ private static ImmutableList runRemotely( return results; } } - throw new RuntimeException( - String.format( - "executorCreated not yet called or no default dynamic_remote_strategy set for %s", - spawn.getMnemonic())); + throw new AssertionError("canExec passed but no usable remote strategy for action " + spawn); } /** diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 6bb72f71e1e1e9..d4deeda9d4e51b 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -1044,6 +1044,7 @@ message DynamicExecution { XCODE_RELATED_PREREQ_UNMET = 1 [(metadata) = { exit_code: 36 }]; ACTION_LOG_MOVE_FAILURE = 2 [(metadata) = { exit_code: 1 }]; RUN_FAILURE = 3 [(metadata) = { exit_code: 1 }]; + NO_USABLE_STRATEGY_FOUND = 4 [(metadata) = { exit_code: 2 }]; } Code code = 1; diff --git a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java index 995f716c0a071e..93b297bfc5e4e1 100644 --- a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyTest.java @@ -125,14 +125,22 @@ private class MockSpawnStrategy implements SandboxedSpawnStrategy { private final DoExec doExecAfterStop; + private final boolean canExec; + MockSpawnStrategy(String name) { this(name, DoExec.NOTHING, DoExec.NOTHING); } MockSpawnStrategy(String name, DoExec doExecBeforeStop, DoExec doExecAfterStop) { + this(name, doExecBeforeStop, doExecAfterStop, true); + } + + MockSpawnStrategy( + String name, DoExec doExecBeforeStop, DoExec doExecAfterStop, boolean canExec) { this.name = name; this.doExecBeforeStop = doExecBeforeStop; this.doExecAfterStop = doExecAfterStop; + this.canExec = canExec; } /** Helper to record an execution failure from within {@link #doExecBeforeStop}. */ @@ -189,7 +197,7 @@ public ImmutableList exec( @Override public boolean canExec(Spawn spawn, ActionContext.ActionContextRegistry actionContextRegistry) { - return true; + return canExec; } @Nullable @@ -552,6 +560,48 @@ public void actionSucceedsIfRemoteExecutionSucceedsEvenIfLocalFailsLater() throw assertThat(outErr.outAsLatin1()).doesNotContain("MockLocalSpawnStrategy"); } + @Test + public void actionSucceedsIfLocalExecutionSucceedsEvenIfRemoteRunsNothing() throws Exception { + MockSpawnStrategy localStrategy = new MockSpawnStrategy("MockLocalSpawnStrategy"); + + MockSpawnStrategy remoteStrategy = + new MockSpawnStrategy("MockRemoteSpawnStrategy", DoExec.NOTHING, DoExec.NOTHING, false); + + StrategyAndContext strategyAndContext = createSpawnStrategy(localStrategy, remoteStrategy); + + Spawn spawn = newDynamicSpawn(); + strategyAndContext.exec(spawn); + + assertThat(localStrategy.getExecutedSpawn()).isEqualTo(spawn); + assertThat(localStrategy.succeeded()).isTrue(); + assertThat(remoteStrategy.getExecutedSpawn()).isNull(); + assertThat(remoteStrategy.succeeded()).isFalse(); + + assertThat(outErr.outAsLatin1()).contains("output files written with MockLocalSpawnStrategy"); + assertThat(outErr.outAsLatin1()).doesNotContain("MockRemoteSpawnStrategy"); + } + + @Test + public void actionSucceedsIfRemoteExecutionSucceedsEvenIfLocalRunsNothing() throws Exception { + MockSpawnStrategy localStrategy = + new MockSpawnStrategy("MockLocalSpawnStrategy", DoExec.NOTHING, DoExec.NOTHING, false); + + MockSpawnStrategy remoteStrategy = new MockSpawnStrategy("MockRemoteSpawnStrategy"); + + StrategyAndContext strategyAndContext = createSpawnStrategy(localStrategy, remoteStrategy); + + Spawn spawn = newDynamicSpawn(); + strategyAndContext.exec(spawn); + + assertThat(localStrategy.getExecutedSpawn()).isNull(); + assertThat(localStrategy.succeeded()).isFalse(); + assertThat(remoteStrategy.getExecutedSpawn()).isEqualTo(spawn); + assertThat(remoteStrategy.succeeded()).isTrue(); + + assertThat(outErr.outAsLatin1()).contains("output files written with MockRemoteSpawnStrategy"); + assertThat(outErr.outAsLatin1()).doesNotContain("MockLocalSpawnStrategy"); + } + @Test public void actionFailsIfLocalFailsImmediatelyEvenIfRemoteSucceedsLater() throws Exception { CountDownLatch countDownLatch = new CountDownLatch(2); @@ -662,6 +712,34 @@ public void actionFailsIfLocalAndRemoteFail() throws Exception { assertThat(remoteStrategy.succeeded()).isFalse(); } + @Test + public void actionFailsIfLocalAndRemoteRunNothing() throws Exception { + MockSpawnStrategy localStrategy = + new MockSpawnStrategy("MockLocalSpawnStrategy", DoExec.NOTHING, DoExec.NOTHING, false); + + MockSpawnStrategy remoteStrategy = + new MockSpawnStrategy("MockRemoteSpawnStrategy", DoExec.NOTHING, DoExec.NOTHING, false); + + StrategyAndContext strategyAndContext = createSpawnStrategy(localStrategy, remoteStrategy); + + Spawn spawn = newDynamicSpawn(); + ExecException e = assertThrows(UserExecException.class, () -> strategyAndContext.exec(spawn)); + + // Has "No usable", followed by both dynamic_local_strategy and dynamic_remote_strategy in, + // followed by the action's mnemonic. + String regexMatch = + "[nN]o usable\\b.*\\bdynamic_local_strategy\\b.*\\bdynamic_remote_strategy\\b.*\\b" + + spawn.getMnemonic() + + "\\b"; + + assertThat(e).hasMessageThat().containsMatch(regexMatch); + + assertThat(localStrategy.getExecutedSpawn()).isNull(); + assertThat(localStrategy.succeeded()).isFalse(); + assertThat(remoteStrategy.getExecutedSpawn()).isNull(); + assertThat(remoteStrategy.succeeded()).isFalse(); + } + @Test public void stopConcurrentSpawnsWaitForCompletion() throws Exception { CountDownLatch countDownLatch = new CountDownLatch(2); diff --git a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyUnitTest.java b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyUnitTest.java index 6623a09e1df3bf..2795a2f6b0be53 100644 --- a/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyUnitTest.java +++ b/src/test/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategyUnitTest.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; +import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.exec.ExecutionPolicy; import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.server.FailureDetails.Execution; @@ -74,6 +75,21 @@ public class DynamicSpawnStrategyUnitTest { private ExecutorService executorServiceForCleanup; + /** + * {@see org.mockito.Mockito#verifyZeroInteractions} + * + *

TODO(b/188373809): {@link org.mockito.Mockito#verifyZeroInteractions + * Mockito.verifyZeroInteractions} is deprecated in Mockito 3.0.1, replaced with {@code + * verifyNoInteractions}. However, some of the builders on Google Bazel Presubmits on BuildKite + * have an older version of that, so we can't replace these calls yet. This function will serve to + * mute the depcrecation warnings until they are. At which point this method should be removed and + * {@code verifyNoInteractions} just be called instead. + */ + @SuppressWarnings("deprecation") + private static void verifyNoInteractions(Object... objs) { + verifyZeroInteractions(objs); + } + @Mock private Function> mockGetPostProcessingSpawn; private Scratch scratch; @@ -86,6 +102,9 @@ public void initMocks() throws IOException { execDir = scratch.dir("/base/exec"); rootDir = ArtifactRoot.asDerivedRoot(execDir, RootType.Output, "root"); MockitoAnnotations.initMocks(this); + // Mockito can't see that we want the function to return Optional.empty() instead + // of null on apply by default (thanks generic type erasure). Set that up ourselves. + when(mockGetPostProcessingSpawn.apply(any())).thenReturn(Optional.empty()); } @After @@ -100,7 +119,7 @@ public void stopExecutorService() throws InterruptedException { } @Test - public void exec_remoteOnlySpawn_doesNotGetLocalPostProcessingSpawn() throws Exception { + public void exec_remoteOnlySpawn_doesNotExecLocalPostProcessingSpawn() throws Exception { DynamicSpawnStrategy dynamicSpawnStrategy = createDynamicSpawnStrategy( ExecutionPolicy.REMOTE_EXECUTION_ONLY, mockGetPostProcessingSpawn); @@ -120,6 +139,28 @@ public void exec_remoteOnlySpawn_doesNotGetLocalPostProcessingSpawn() throws Exc assertThat(remoteSpawnCaptor.getAllValues()).containsExactly(spawn); } + @Test + public void exec_remoteOnlySpawn_noneCanExec_fails() throws Exception { + Spawn spawn = new SpawnBuilder().withMnemonic("TheThing").build(); + DynamicSpawnStrategy dynamicSpawnStrategy = + createDynamicSpawnStrategy( + ExecutionPolicy.REMOTE_EXECUTION_ONLY, mockGetPostProcessingSpawn); + SandboxedSpawnStrategy local = createMockSpawnStrategy(); + SandboxedSpawnStrategy remote = createMockSpawnStrategy(false); + ActionExecutionContext actionExecutionContext = createMockActionExecutionContext(local, remote); + + UserExecException thrown = + assertThrows( + UserExecException.class, + () -> dynamicSpawnStrategy.exec(spawn, actionExecutionContext)); + assertThat(thrown).hasMessageThat().doesNotContain("dynamic_local_strategy"); + assertThat(thrown).hasMessageThat().containsMatch("\\bdynamic_remote_strategy\\b"); + assertThat(thrown).hasMessageThat().containsMatch("\\bTheThing\\b"); + verifyNoInteractions(local); + // No post processing because local never ran. + verify(mockGetPostProcessingSpawn, never()).apply(any()); + } + @Test public void exec_localOnlySpawn_runsLocalPostProcessingSpawn() throws Exception { Spawn spawn = new SpawnBuilder("command").build(); @@ -137,15 +178,15 @@ public void exec_localOnlySpawn_runsLocalPostProcessingSpawn() throws Exception ImmutableList results = dynamicSpawnStrategy.exec(spawn, actionExecutionContext); assertThat(results).containsExactly(SUCCESSFUL_SPAWN_RESULT, SUCCESSFUL_SPAWN_RESULT); - verifyZeroInteractions(remote); + verifyNoInteractions(remote); assertThat(localSpawnCaptor.getAllValues()) .containsExactly(spawn, postProcessingSpawn) .inOrder(); } @Test - public void exec_failedLocalSpawn_doesNotGetLocalPostProcessingSpawn() throws Exception { - testExecFailedLocalSpawnDoesNotGetLocalPostProcessingSpawn( + public void exec_failedLocalSpawn_doesNotExecLocalPostProcessingSpawn() throws Exception { + testExecFailedLocalSpawnDoesNotExecLocalPostProcessingSpawn( new SpawnResult.Builder() .setRunnerName("test") .setStatus(Status.TIMEOUT) @@ -155,8 +196,31 @@ public void exec_failedLocalSpawn_doesNotGetLocalPostProcessingSpawn() throws Ex } @Test - public void exec_nonZeroExitCodeLocalSpawn_doesNotGetLocalPostProcessingSpawn() throws Exception { - testExecFailedLocalSpawnDoesNotGetLocalPostProcessingSpawn( + public void exec_localOnlySpawn_noneCanExec_fails() throws Exception { + Spawn spawn = new SpawnBuilder().withMnemonic("TheThing").build(); + DynamicSpawnStrategy dynamicSpawnStrategy = + createDynamicSpawnStrategy( + ExecutionPolicy.LOCAL_EXECUTION_ONLY, mockGetPostProcessingSpawn); + SandboxedSpawnStrategy local = createMockSpawnStrategy(false); + SandboxedSpawnStrategy remote = createMockSpawnStrategy(); + ActionExecutionContext actionExecutionContext = createMockActionExecutionContext(local, remote); + + UserExecException thrown = + assertThrows( + UserExecException.class, + () -> dynamicSpawnStrategy.exec(spawn, actionExecutionContext)); + assertThat(thrown).hasMessageThat().containsMatch("\\bdynamic_local_strategy\\b"); + assertThat(thrown).hasMessageThat().doesNotContain("dynamic_remote_strategy"); + assertThat(thrown).hasMessageThat().containsMatch("\\bTheThing\\b"); + verifyNoInteractions(remote); + // No post processing because local never completed. + verify(mockGetPostProcessingSpawn, never()).apply(any()); + } + + @Test + public void exec_nonZeroExitCodeLocalSpawn_doesNotExecLocalPostProcessingSpawn() + throws Exception { + testExecFailedLocalSpawnDoesNotExecLocalPostProcessingSpawn( new SpawnResult.Builder() .setRunnerName("test") .setStatus(Status.EXECUTION_FAILED) @@ -165,7 +229,7 @@ public void exec_nonZeroExitCodeLocalSpawn_doesNotGetLocalPostProcessingSpawn() .build()); } - private void testExecFailedLocalSpawnDoesNotGetLocalPostProcessingSpawn(SpawnResult failedResult) + private void testExecFailedLocalSpawnDoesNotExecLocalPostProcessingSpawn(SpawnResult failedResult) throws Exception { DynamicSpawnStrategy dynamicSpawnStrategy = createDynamicSpawnStrategy( @@ -183,7 +247,6 @@ private void testExecFailedLocalSpawnDoesNotGetLocalPostProcessingSpawn(SpawnRes assertThat(results).containsExactly(failedResult); assertThat(localSpawnCaptor.getAllValues()).containsExactly(spawn); verify(remote, never()).exec(any(), any(), any()); - verify(mockGetPostProcessingSpawn, never()).apply(any()); } @Test @@ -238,6 +301,82 @@ public void waitBranches_givesDebugOutputOnWeirdCases() throws Exception { assertThat(error).hasMessageThat().contains("Neither branch of /foo completed."); } + @Test + public void exec_runAnywhereSpawn_localCantExec_runsRemote() throws Exception { + Spawn spawn = new SpawnBuilder().build(); + DynamicSpawnStrategy dynamicSpawnStrategy = + createDynamicSpawnStrategy(ExecutionPolicy.ANYWHERE, mockGetPostProcessingSpawn); + SandboxedSpawnStrategy local = createMockSpawnStrategy(false); + SandboxedSpawnStrategy remote = createMockSpawnStrategy(); + when(remote.exec(eq(spawn), any(), any())) + .thenAnswer( + invocation -> { + StopConcurrentSpawns stopConcurrentSpawns = invocation.getArgument(2); + if (stopConcurrentSpawns != null) { + stopConcurrentSpawns.stop(); + } + return ImmutableList.of(SUCCESSFUL_SPAWN_RESULT); + }); + ActionExecutionContext actionExecutionContext = createMockActionExecutionContext(local, remote); + + ImmutableList results = dynamicSpawnStrategy.exec(spawn, actionExecutionContext); + + assertThat(results).containsExactly(SUCCESSFUL_SPAWN_RESULT); + // Never runs anything as it says it can't execute anything at all. + verify(local, never()).exec(any(), any(), any()); + verify(mockGetPostProcessingSpawn, never()).apply(any()); + } + + @Test + public void exec_runAnywhereSpawn_remoteCantExec_runsLocal() throws Exception { + Spawn spawn = new SpawnBuilder().build(); + Spawn postProcessingSpawn = new SpawnBuilder("extra_command").build(); + DynamicSpawnStrategy dynamicSpawnStrategy = + createDynamicSpawnStrategy( + ExecutionPolicy.ANYWHERE, ignored -> Optional.of(postProcessingSpawn)); + SandboxedSpawnStrategy local = createMockSpawnStrategy(); + ArgumentCaptor localSpawnCaptor = ArgumentCaptor.forClass(Spawn.class); + when(local.exec(localSpawnCaptor.capture(), any(), any())) + .thenAnswer( + invocation -> { + StopConcurrentSpawns stopConcurrentSpawns = invocation.getArgument(2); + if (stopConcurrentSpawns != null) { + stopConcurrentSpawns.stop(); + } + return ImmutableList.of(SUCCESSFUL_SPAWN_RESULT); + }); + SandboxedSpawnStrategy remote = createMockSpawnStrategy(false); + ActionExecutionContext actionExecutionContext = createMockActionExecutionContext(local, remote); + + ImmutableList results = dynamicSpawnStrategy.exec(spawn, actionExecutionContext); + + assertThat(results).containsExactly(SUCCESSFUL_SPAWN_RESULT, SUCCESSFUL_SPAWN_RESULT); + assertThat(localSpawnCaptor.getAllValues()) + .containsExactly(spawn, postProcessingSpawn) + .inOrder(); + verify(remote, never()).exec(any(), any(), any()); + } + + @Test + public void exec_runAnywhereSpawn_noneCanExec_fails() throws Exception { + Spawn spawn = new SpawnBuilder().withMnemonic("TheThing").build(); + DynamicSpawnStrategy dynamicSpawnStrategy = + createDynamicSpawnStrategy(ExecutionPolicy.ANYWHERE, mockGetPostProcessingSpawn); + SandboxedSpawnStrategy local = createMockSpawnStrategy(false); + SandboxedSpawnStrategy remote = createMockSpawnStrategy(false); + ActionExecutionContext actionExecutionContext = createMockActionExecutionContext(local, remote); + + UserExecException thrown = + assertThrows( + UserExecException.class, + () -> dynamicSpawnStrategy.exec(spawn, actionExecutionContext)); + assertThat(thrown).hasMessageThat().containsMatch("\\bdynamic_local_strategy\\b"); + assertThat(thrown).hasMessageThat().containsMatch("\\bdynamic_remote_strategy\\b"); + assertThat(thrown).hasMessageThat().containsMatch("\\bTheThing\\b"); + // No post processing because local never completed. + verify(mockGetPostProcessingSpawn, never()).apply(any()); + } + private DynamicSpawnStrategy createDynamicSpawnStrategy( ExecutionPolicy executionPolicy, Function> getPostProcessingSpawnForLocalExecution) { @@ -279,8 +418,13 @@ public void notifyUsedDynamic(ActionContextRegistry actionContextRegistry) {} } private static SandboxedSpawnStrategy createMockSpawnStrategy() throws InterruptedException { + return createMockSpawnStrategy(true); + } + + private static SandboxedSpawnStrategy createMockSpawnStrategy(boolean canExec) + throws InterruptedException { SandboxedSpawnStrategy strategy = mock(SandboxedSpawnStrategy.class); - when(strategy.canExec(any(), any())).thenReturn(true); + when(strategy.canExec(any(), any())).thenReturn(canExec); when(strategy.beginExecution(any(), any())).thenThrow(UnsupportedOperationException.class); return strategy; }