Skip to content

Commit

Permalink
Fix StandaloneTestStrategy.appendStderr
Browse files Browse the repository at this point in the history
As of 4a5e1b7, it was using getErrorPath twice, which could cause it to
loop indefinitely, trying to append the error file to itself. This
happened rarely, as the test runner script redirects stderr to stdout.
However, it could happen if the SpawnRunner wrote any extra output to
stderr, which the RemoteSpawnRunner does in some cases.

I have manually checked that this fixes the issue, and also added a
regression test.

Fixes #8320.

PiperOrigin-RevId: 249258656
  • Loading branch information
ulfjack authored and copybara-github committed May 21, 2019
1 parent 6c0dc36 commit 0ff19c6
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ private static void appendStderr(FileOutErr outErr) throws IOException {
if (stat != null) {
try {
if (stat.getSize() > 0) {
Path stdOut = outErr.getErrorPath();
Path stdOut = outErr.getOutputPath();
if (stdOut.exists()) {
stdOut.setWritable(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -655,4 +655,49 @@ public void testEmptyOutputCreatesEmptyLogFile() throws Exception {
}
assertThat(outErr.getErrorPath().exists()).isFalse();
}

@Test
public void testAppendStdErrDoesNotBusyLoop() throws Exception {
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
executionOptions.testOutput = TestOutputFormat.ALL;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);

// setup a test action
scratch.file("standalone/empty_test.sh", "this does not get executed, it is mocked out");
scratch.file(
"standalone/BUILD",
"sh_test(",
" name = \"empty_test\",",
" size = \"small\",",
" srcs = [\"empty_test.sh\"],",
")");
TestRunnerAction testRunnerAction = getTestAction("//standalone:empty_test");

SpawnResult expectedSpawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
when(spawnActionContext.beginExecution(any(), any()))
.then(
(invocation) -> {
((ActionExecutionContext) invocation.getArgument(1)).getFileOutErr().printErr("Foo");
return SpawnContinuation.immediate(expectedSpawnResult);
});

FileOutErr outErr = createTempOutErr(tmpDirRoot);
ActionExecutionContext actionExecutionContext =
new FakeActionExecutionContext(outErr, spawnActionContext);

// actual StandaloneTestStrategy execution
execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);

// check that the test stdout contains all the expected output
try {
String outData = FileSystemUtils.readContent(outErr.getOutputPath(), UTF_8);
assertThat(outData).contains("Foo");
} catch (IOException e) {
fail("Test stdout file missing: " + outErr.getOutputPath());
}
}
}

0 comments on commit 0ff19c6

Please sign in to comment.