Skip to content

Commit

Permalink
Write stdout and stderr to the remote cache
Browse files Browse the repository at this point in the history
Only write a cache entry when the spawn executed successfully, and with a 0
exit code. In the test, we only check that uploadFileContents is called exactly
twice.

Progress on #1413.

PiperOrigin-RevId: 153458240
  • Loading branch information
ulfjack authored and aehlig committed Apr 18, 2017
1 parent d1b34d4 commit 7a90b9a
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ public SpawnResult exec(
}
}
SpawnResult spawnResult = delegate.exec(spawn, policy);
if (options.remoteLocalExecUploadResults && spawnResult.setupSuccess()) {
writeCacheEntry(spawn, actionKey);
if (options.remoteLocalExecUploadResults
&& spawnResult.status() == Status.SUCCESS
&& spawnResult.exitCode() == 0) {
writeCacheEntry(spawn, policy.getFileOutErr(), actionKey);
}
return spawnResult;
} catch (StatusRuntimeException e) {
Expand Down Expand Up @@ -172,7 +174,7 @@ private void passRemoteOutErr(
outErr.printErr(new String(streams.get(1), UTF_8));
}

private void writeCacheEntry(Spawn spawn, ActionKey actionKey)
private void writeCacheEntry(Spawn spawn, FileOutErr outErr, ActionKey actionKey)
throws IOException, InterruptedException {
ArrayList<Path> outputFiles = new ArrayList<>();
for (ActionInput output : spawn.getOutputFiles()) {
Expand All @@ -185,6 +187,10 @@ private void writeCacheEntry(Spawn spawn, ActionKey actionKey)
}
ActionResult.Builder result = ActionResult.newBuilder();
actionCache.uploadAllResults(execRoot, outputFiles, result);
ContentDigest stderr = actionCache.uploadFileContents(outErr.getErrorPath());
ContentDigest stdout = actionCache.uploadFileContents(outErr.getOutputPath());
result.setStderrDigest(stderr);
result.setStdoutDigest(stdout);
actionCache.setCachedActionResult(actionKey, result.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public void expand(Artifact artifact, Collection<? super Artifact> output) {
}
};

private static final ContentDigest DIGEST_FOR_EMPTY = ContentDigests.computeDigest(new byte[0]);

private FileSystem fs;
private Path execRoot;
private SimpleSpawn simpleSpawn;
Expand Down Expand Up @@ -219,6 +221,7 @@ public void cacheMiss() throws Exception {
scratch(simpleSpawn.getInputFiles().get(0), "xyz");

when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null);
when(cache.uploadFileContents(any(Path.class))).thenReturn(DIGEST_FOR_EMPTY);
SpawnResult delegateResult = new SpawnResult.Builder()
.setExitCode(0)
.setStatus(Status.SUCCESS)
Expand All @@ -227,9 +230,11 @@ public void cacheMiss() throws Exception {
.thenReturn(delegateResult);

SpawnResult result = runner.exec(simpleSpawn, simplePolicy);
// We use verify to check that each method is called exactly once.
// We use verify to check that each method is called the correct number of times.
verify(cache)
.uploadAllResults(any(Path.class), any(Collection.class), any(ActionResult.Builder.class));
// Two additional uploads for stdout and stderr.
verify(cache, Mockito.times(2)).uploadFileContents(any(Path.class));
verify(cache).setCachedActionResult(any(ActionKey.class), any(ActionResult.class));
assertThat(result.setupSuccess()).isTrue();
assertThat(result.exitCode()).isEqualTo(0);
Expand Down

0 comments on commit 7a90b9a

Please sign in to comment.