Skip to content

Commit

Permalink
more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@SrodriguezO
SrodriguezO committed Jun 27, 2019
1 parent 03a6640 commit f0ac726
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 48 deletions.
33 changes: 18 additions & 15 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,10 @@ public String getName() {
@Override
public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
throws ExecException, InterruptedException, IOException {
if (!Spawns.mayBeExecutedRemotely(spawn)) {
return execLocally(spawn, context);
}

boolean spawnCachable = Spawns.mayBeCachedRemotely(spawn);
boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults && spawnCachable;
boolean acceptCachedResult = remoteOptions.remoteAcceptCached && spawnCachable;

context.report(ProgressStatus.EXECUTING, getName());
RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode;
Expand All @@ -177,7 +177,6 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
// Get the remote platform properties.
Platform platform =
parsePlatform(spawn.getExecutionPlatform(), remoteOptions.remoteDefaultPlatformProperties);

Command command =
buildCommand(
spawn.getOutputFiles(), spawn.getArguments(), spawn.getEnvironment(), platform);
Expand All @@ -190,15 +189,17 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
spawnCachable);
ActionKey actionKey = digestUtil.computeActionKey(action);

if (!Spawns.mayBeExecutedRemotely(spawn)) {
return execLocallyAndUpload(
spawn, context, inputMap, actionKey, action, command, uploadLocalResults);
}

// Look up action cache, and reuse the action output if it is found.
Context withMetadata =
TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey);
Context previous = withMetadata.attach();
Profiler prof = Profiler.instance();
try {
boolean acceptCachedResult = remoteOptions.remoteAcceptCached && spawnCachable;
boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults && spawnCachable;

try {
// Try to lookup the action in the action cache.
ActionResult cachedResult;
Expand Down Expand Up @@ -230,7 +231,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
if (remoteExecutor == null) {
// Remote execution is disabled and so execute the spawn on the local machine.
return execLocallyAndUpload(
spawn, context, inputMap, remoteCache, actionKey, action, command, uploadLocalResults);
spawn, context, inputMap, actionKey, action, command, uploadLocalResults);
}

ExecuteRequest.Builder requestBuilder =
Expand All @@ -254,12 +255,15 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
ExecuteRequest request = requestBuilder.build();

// Upload the command and all the inputs into the remote cache.
try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) {
Map<Digest, Message> additionalInputs = Maps.newHashMapWithExpectedSize(2);
additionalInputs.put(actionKey.getDigest(), action);
additionalInputs.put(commandHash, command);
remoteCache.ensureInputsPresent(merkleTree, additionalInputs, execRoot);
if (spawnCachable) {
try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) {
Map<Digest, Message> additionalInputs = Maps.newHashMapWithExpectedSize(2);
additionalInputs.put(actionKey.getDigest(), action);
additionalInputs.put(commandHash, command);
remoteCache.ensureInputsPresent(merkleTree, additionalInputs, execRoot);
}
}

ExecuteResponse reply;
try (SilentCloseable c = prof.profile(REMOTE_EXECUTION, "execute remotely")) {
reply = remoteExecutor.executeRemotely(request);
Expand Down Expand Up @@ -403,7 +407,7 @@ private SpawnResult execLocallyAndUploadOrFail(
}
if (remoteOptions.remoteLocalFallback && !RemoteRetrierUtils.causedByExecTimeout(cause)) {
return execLocallyAndUpload(
spawn, context, inputMap, remoteCache, actionKey, action, command, uploadLocalResults);
spawn, context, inputMap, actionKey, action, command, uploadLocalResults);
}
return handleError(cause, context.getFileOutErr(), actionKey);
}
Expand Down Expand Up @@ -563,7 +567,6 @@ SpawnResult execLocallyAndUpload(
Spawn spawn,
SpawnExecutionContext context,
SortedMap<PathFragment, ActionInput> inputMap,
AbstractRemoteActionCache remoteCache,
ActionKey actionKey,
Action action,
Command command,
Expand Down
5 changes: 2 additions & 3 deletions src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyCollection;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -413,15 +412,15 @@ public void noRemoteCacheStillUsesLocalCache() throws Exception {
SimpleSpawn cacheableSpawn =
simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_CACHE, ""));
cache.lookup(cacheableSpawn, simplePolicy);
verify(remoteCache, atLeastOnce()).getCachedActionResult(any(ActionKey.class));
verify(remoteCache).getCachedActionResult(any(ActionKey.class));
}

@Test
public void noRemoteExecStillUsesCache() throws Exception {
SimpleSpawn cacheableSpawn =
simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_EXEC, ""));
cache.lookup(cacheableSpawn, simplePolicy);
verify(remoteCache, atLeastOnce()).getCachedActionResult(any(ActionKey.class));
verify(remoteCache).getCachedActionResult(any(ActionKey.class));
}

@Test
Expand Down
126 changes: 96 additions & 30 deletions src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,7 @@ public void nonCachableSpawnsShouldNotBeCached_remote() throws Exception {
.build();
when(executor.executeRemotely(any(ExecuteRequest.class))).thenReturn(succeeded);

Spawn spawn =
new SimpleSpawn(
new FakeOwner("foo", "bar"),
/*arguments=*/ ImmutableList.of(),
/*environment=*/ ImmutableMap.of(),
NO_CACHE,
/*inputs=*/ ImmutableList.of(),
/*outputs=*/ ImmutableList.<ActionInput>of(),
ResourceSet.ZERO);

Spawn spawn = simpleSpawnWithExecutionInfo(NO_CACHE);
SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn);

runner.exec(spawn, policy);
Expand All @@ -213,39 +204,109 @@ public void nonCachableSpawnsShouldNotBeCached_remote() throws Exception {
}

@Test
public void nonCachableSpawnsShouldNotBeCached_local() throws Exception {
// Test that if a spawn is executed locally, due to the local fallback, that its result is not
// uploaded to the remote cache.
public void nonCachableSpawnsShouldNotBeCached_localFallback() throws Exception {
// Test that if a non-cachable spawn is executed locally due to the local fallback,
// that its result is not uploaded to the remote cache.

remoteOptions.remoteAcceptCached = true;
remoteOptions.remoteLocalFallback = true;
remoteOptions.remoteUploadLocalResults = true;

RemoteSpawnRunner runner = newSpawnRunnerWithoutExecutor();
RemoteSpawnRunner runner = newSpawnRunner();

// Throw an IOException to trigger the local fallback.
when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(IOException.class);

Spawn spawn =
new SimpleSpawn(
new FakeOwner("foo", "bar"),
/*arguments=*/ ImmutableList.of(),
/*environment=*/ ImmutableMap.of(),
NO_CACHE,
/*inputs=*/ ImmutableList.of(),
/*outputs=*/ ImmutableList.<ActionInput>of(),
ResourceSet.ZERO);

Spawn spawn = simpleSpawnWithExecutionInfo(NO_CACHE);
SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn);

runner.exec(spawn, policy);

verify(localRunner).exec(spawn, policy);

verify(cache, never()).getCachedActionResult(any(ActionKey.class));
verify(cache, never()).upload(any(), any(), any(), any(), any(), any());
verifyNoMoreInteractions(cache);
}

@Test
public void cachableSpawnsShouldBeCached_localFallback() throws Exception {
// Test that if a cachable spawn is executed locally due to the local fallback,
// that its result is uploaded to the remote cache.

remoteOptions.remoteAcceptCached = true;
remoteOptions.remoteLocalFallback = true;
remoteOptions.remoteUploadLocalResults = true;

RemoteSpawnRunner runner = spy(newSpawnRunner());

// Throw an IOException to trigger the local fallback.
when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(IOException.class);

SpawnResult res =
new SpawnResult.Builder()
.setStatus(Status.SUCCESS)
.setExitCode(0)
.setRunnerName("test")
.build();
when(localRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(res);

Spawn spawn = newSimpleSpawn();
SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn);

SpawnResult result = runner.exec(spawn, policy);
assertThat(result.exitCode()).isEqualTo(0);
assertThat(result.status()).isEqualTo(Status.SUCCESS);
verify(localRunner).exec(eq(spawn), eq(policy));
verify(runner)
.execLocallyAndUpload(
eq(spawn),
eq(policy),
any(),
any(),
any(),
any(),
/* uploadLocalResults= */ eq(true));
verify(cache).upload(any(), any(), any(), any(), any(), any());
}

@Test
public void noRemoteExecExecutesLocallyButUsesCache() throws Exception {
// Test that if the NO_REMOTE_EXEC execution requirement is specified, the spawn is executed
// locally, but its result is still uploaded to the remote cache.

remoteOptions.remoteAcceptCached = true;
remoteOptions.remoteUploadLocalResults = true;

RemoteSpawnRunner runner = spy(newSpawnRunner());

SpawnResult res =
new SpawnResult.Builder()
.setStatus(Status.SUCCESS)
.setExitCode(0)
.setRunnerName("test")
.build();
when(localRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(res);

Spawn spawn =
simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_EXEC, ""));
SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn);

SpawnResult result = runner.exec(spawn, policy);

assertThat(result.exitCode()).isEqualTo(0);
assertThat(result.status()).isEqualTo(Status.SUCCESS);
verify(localRunner).exec(eq(spawn), eq(policy));
verify(runner)
.execLocallyAndUpload(
eq(spawn),
eq(policy),
any(),
any(),
any(),
any(),
/* uploadLocalResults= */ eq(true));
verify(cache).upload(any(), any(), any(), any(), any(), any());
}

@Test
public void failedLocalActionShouldNotBeUploaded() throws Exception {
// Test that the outputs of a locally executed action that failed are not uploaded.
Expand All @@ -270,7 +331,6 @@ public void failedLocalActionShouldNotBeUploaded() throws Exception {
eq(spawn),
eq(policy),
any(),
eq(cache),
any(),
any(),
any(),
Expand Down Expand Up @@ -307,7 +367,6 @@ public void treatFailedCachedActionAsCacheMiss_local() throws Exception {
eq(spawn),
eq(policy),
any(),
eq(cache),
any(),
any(),
any(),
Expand Down Expand Up @@ -794,7 +853,7 @@ public void testMaterializeParamFiles() throws Exception {
/*arguments=*/ ImmutableList.of(),
/*environment=*/ ImmutableMap.of(),
/*executionInfo=*/ ImmutableMap.of(),
ImmutableList.of(input),
/*inputs=*/ ImmutableList.of(input),
/*outputs=*/ ImmutableList.<ActionInput>of(),
ResourceSet.ZERO);
SpawnExecutionContext policy = new FakeSpawnExecutionContext(spawn);
Expand Down Expand Up @@ -934,11 +993,18 @@ public void testDownloadTopLevel() throws Exception {
}

private static Spawn newSimpleSpawn(Artifact... outputs) {
return simpleSpawnWithExecutionInfo(ImmutableMap.of(), outputs);
}

private static SimpleSpawn simpleSpawnWithExecutionInfo(
ImmutableMap<String, String> executionInfo,
Artifact... outputs
) {
return new SimpleSpawn(
new FakeOwner("foo", "bar"),
/*arguments=*/ ImmutableList.of(),
/*environment=*/ ImmutableMap.of(),
/*executionInfo=*/ ImmutableMap.of(),
/*executionInfo=*/ executionInfo,
/*inputs=*/ ImmutableList.of(),
/*outputs=*/ ImmutableList.copyOf(outputs),
ResourceSet.ZERO);
Expand Down

0 comments on commit f0ac726

Please sign in to comment.