Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Commit

Permalink
Improve error handling and include stderr in XctoolRunTestsStep
Browse files Browse the repository at this point in the history 
Summary:
I just upgraded Xcode on my local dev server. When I did,
`buck test` started failing because the SDKs weren't installed
correctly.

It happened that `xctool` was the first thing to fail in my case,
but we didn't include stderr when it failed.

This updates `XctoolRunTestsStep` to include stderr when it fails.

Test Plan: New tests added. `ant java-test -Dtest.class=XctoolRunTestsStepTest`

Reviewed By: ryu2

fb-gh-sync-id: a6228f3
  • Loading branch information
@bhamiltoncx
bhamiltoncx authored and facebook-github-bot-6 committed Jan 15, 2016
1 parent 3e714fd commit 3c42b08
Show file tree
Hide file tree
Showing 2 changed files with 194 additions and 25 deletions.
92 changes: 71 additions & 21 deletions src/com/facebook/buck/apple/XctoolRunTestsStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.facebook.buck.step.Step;
import com.facebook.buck.test.selectors.TestDescription;
import com.facebook.buck.test.selectors.TestSelectorList;
import com.facebook.buck.util.Console;
import com.facebook.buck.util.Escaper;
import com.facebook.buck.util.MoreThrowables;
import com.facebook.buck.util.ProcessExecutor;
Expand All @@ -37,6 +38,7 @@
import com.google.common.collect.Multimap;
import com.google.common.collect.Iterables;
import com.google.common.io.ByteStreams;
import com.google.common.io.CharStreams;

import java.io.BufferedReader;
import java.io.IOException;
Expand All @@ -47,7 +49,7 @@
import java.nio.file.Path;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
Expand Down Expand Up @@ -244,17 +246,26 @@ public int execute(ExecutionContext context) throws InterruptedException {

if (!testSelectorList.isEmpty()) {
try {
List<String> xctoolFilterParams = listAndFilterTestsThenFormatXctoolParams(
ImmutableList.Builder<String> xctoolFilterParamsBuilder = ImmutableList.builder();
int returnCode = listAndFilterTestsThenFormatXctoolParams(
context.getProcessExecutor(),
context.getConsole(),
testSelectorList,
// Copy the entire xctool command and environment but add a -listTestsOnly arg.
ProcessExecutorParams.builder()
.from(processExecutorParamsBuilder.build())
.addCommand("-listTestsOnly")
.build());
.build(),
xctoolFilterParamsBuilder);
if (returnCode != 0) {
context.getConsole().printErrorText("Failed to query tests with xctool");
return returnCode;
}
ImmutableList<String> xctoolFilterParams = xctoolFilterParamsBuilder.build();
if (xctoolFilterParams.isEmpty()) {
context.getConsole().printBuildFailure(
String.format(
Locale.US,
"No tests found matching specified filter (%s)",
testSelectorList.getExplanation()));
return 0;
Expand Down Expand Up @@ -282,10 +293,15 @@ public int execute(ExecutionContext context) throws InterruptedException {
context.getProcessExecutor().launchProcess(processExecutorParams);

int exitCode;
String stderr;
try (OutputStream outputStream = filesystem.newFileOutputStream(
outputPath);
TeeInputStream stdoutWrapperStream = new TeeInputStream(
launchedProcess.getInputStream(), outputStream)) {
launchedProcess.getInputStream(), outputStream);
InputStreamReader stderrReader = new InputStreamReader(
launchedProcess.getErrorStream(),
StandardCharsets.UTF_8);
BufferedReader bufferedStderrReader = new BufferedReader(stderrReader)) {
if (stdoutReadingCallback.isPresent()) {
// The caller is responsible for reading all the data, which TeeInputStream will
// copy to outputStream.
Expand All @@ -296,15 +312,29 @@ public int execute(ExecutionContext context) throws InterruptedException {
stdoutWrapperStream.close();
ByteStreams.copy(launchedProcess.getInputStream(), outputStream);
}
stderr = CharStreams.toString(bufferedStderrReader).trim();
exitCode = waitForProcessAndGetExitCode(context.getProcessExecutor(), launchedProcess);
LOG.debug("Finished running command, exit code %d", exitCode);
LOG.debug("Finished running command, exit code %d, stderr %s", exitCode, stderr);
} finally {
context.getProcessExecutor().destroyLaunchedProcess(launchedProcess);
context.getProcessExecutor().waitForLaunchedProcess(launchedProcess);
}

if (exitCode != 0) {
LOG.warn("%s exited with error %d", command, exitCode);
if (!stderr.isEmpty()) {
context.getConsole().printErrorText(
String.format(
Locale.US,
"xctool failed with exit code %d: %s",
exitCode,
stderr));
} else {
context.getConsole().printErrorText(
String.format(
Locale.US,
"xctool failed with exit code %d",
exitCode));
}
}

return exitCode;
Expand All @@ -325,10 +355,12 @@ public String getDescription(ExecutionContext context) {
return Joiner.on(' ').join(Iterables.transform(command, Escaper.SHELL_ESCAPER));
}

private static List<String> listAndFilterTestsThenFormatXctoolParams(
private static int listAndFilterTestsThenFormatXctoolParams(
ProcessExecutor processExecutor,
Console console,
TestSelectorList testSelectorList,
ProcessExecutorParams listTestsOnlyParams) throws IOException, InterruptedException {
ProcessExecutorParams listTestsOnlyParams,
ImmutableList.Builder<String> filterParamsBuilder) throws IOException, InterruptedException {
Preconditions.checkArgument(!testSelectorList.isEmpty());
LOG.debug("Filtering tests with selector list: %s", testSelectorList.getExplanation());

Expand All @@ -337,31 +369,50 @@ private static List<String> listAndFilterTestsThenFormatXctoolParams(
processExecutor.launchProcess(listTestsOnlyParams);

ListTestsOnlyHandler listTestsOnlyHandler = new ListTestsOnlyHandler();
String stderr;
int listTestsResult;
try (InputStreamReader isr =
new InputStreamReader(
launchedProcess.getInputStream(),
StandardCharsets.UTF_8);
BufferedReader br = new BufferedReader(isr)) {
BufferedReader br = new BufferedReader(isr);
InputStreamReader esr =
new InputStreamReader(
launchedProcess.getErrorStream(),
StandardCharsets.UTF_8);
BufferedReader ebr = new BufferedReader(esr)) {
XctoolOutputParsing.streamOutputFromReader(br, listTestsOnlyHandler);
stderr = CharStreams.toString(ebr).trim();
listTestsResult = processExecutor.waitForLaunchedProcess(launchedProcess);
}

int listTestsResult = processExecutor.waitForLaunchedProcess(launchedProcess);
if (listTestsResult != 0) {
throw new IOException(
String.format(
"Process %s exited with error %d",
listTestsOnlyParams.getCommand(),
listTestsResult));
if (!stderr.isEmpty()) {
console.printErrorText(
String.format(
Locale.US,
"xctool failed with exit code %d: %s",
listTestsResult,
stderr));
} else {
console.printErrorText(
String.format(
Locale.US,
"xctool failed with exit code %d",
listTestsResult));
}
} else {
formatXctoolFilterParams(
testSelectorList, listTestsOnlyHandler.testTargetsToDescriptions, filterParamsBuilder);
}

return formatXctoolFilterParams(
testSelectorList, listTestsOnlyHandler.testTargetsToDescriptions);
return listTestsResult;
}

private static List<String> formatXctoolFilterParams(
private static void formatXctoolFilterParams(
TestSelectorList testSelectorList,
Multimap<String, TestDescription> testTargetsToDescriptions) {
ImmutableList.Builder<String> filterParamsBuilder = ImmutableList.builder();
Multimap<String, TestDescription> testTargetsToDescriptions,
ImmutableList.Builder<String> filterParamsBuilder) {
for (String testTarget : testTargetsToDescriptions.keySet()) {
StringBuilder sb = new StringBuilder();
boolean matched = false;
Expand All @@ -385,7 +436,6 @@ private static List<String> formatXctoolFilterParams(
filterParamsBuilder.add(sb.toString());
}
}
return filterParamsBuilder.build();
}

private static ImmutableList<String> createCommandArgs(
Expand Down
127 changes: 123 additions & 4 deletions test/com/facebook/buck/apple/XctoolRunTestsStepTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.facebook.buck.apple;

import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
Expand All @@ -25,6 +27,7 @@
import com.facebook.buck.step.TestExecutionContext;
import com.facebook.buck.test.selectors.TestSelectorList;
import com.facebook.buck.testutil.FakeProjectFilesystem;
import com.facebook.buck.testutil.TestConsole;
import com.facebook.buck.util.FakeProcess;
import com.facebook.buck.util.FakeProcessExecutor;
import com.facebook.buck.util.ProcessExecutorParams;
Expand Down Expand Up @@ -91,6 +94,58 @@ public void xctoolCommandWithOnlyLogicTests() throws Exception {
equalTo(0));
}

@Test
public void xctoolCommandWhichFailsPrintsStderrToConsole() throws Exception {
FakeProjectFilesystem projectFilesystem = new FakeProjectFilesystem();
XctoolRunTestsStep step = new XctoolRunTestsStep(
projectFilesystem,
Paths.get("/path/to/xctool"),
Optional.<Long>absent(),
"iphonesimulator",
Optional.<String>absent(),
ImmutableSet.of(Paths.get("/path/to/Foo.xctest")),
ImmutableMap.<Path, Path>of(),
Paths.get("/path/to/output.json"),
Optional.<XctoolRunTestsStep.StdoutReadingCallback>absent(),
Suppliers.ofInstance(Optional.of(Paths.get("/path/to/developer/dir"))),
TestSelectorList.EMPTY,
Optional.<String>absent(),
Optional.<Path>absent(),
Optional.<String>absent(),
Optional.<String>absent());
ProcessExecutorParams xctoolParams =
ProcessExecutorParams.builder()
.setCommand(
ImmutableList.of(
"/path/to/xctool",
"-reporter",
"json-stream",
"-sdk",
"iphonesimulator",
"run-tests",
"-logicTest",
"/path/to/Foo.xctest"))
.setEnvironment(ImmutableMap.of("DEVELOPER_DIR", "/path/to/developer/dir"))
.setDirectory(projectFilesystem.getRootPath().toAbsolutePath().toFile())
.setRedirectOutput(ProcessBuilder.Redirect.PIPE)
.build();
FakeProcess fakeXctoolFailure = new FakeProcess(42, "", "Something went terribly wrong\n");
FakeProcessExecutor processExecutor = new FakeProcessExecutor(
ImmutableMap.of(xctoolParams, fakeXctoolFailure));
TestConsole testConsole = new TestConsole();
ExecutionContext executionContext = TestExecutionContext.newBuilder()
.setProcessExecutor(processExecutor)
.setEnvironment(ImmutableMap.<String, String>of())
.setConsole(testConsole)
.build();
assertThat(
step.execute(executionContext),
equalTo(42));
assertThat(
testConsole.getTextWrittenToStdErr(),
equalTo("xctool failed with exit code 42: Something went terribly wrong\n"));
}

@Test
public void xctoolCommandWithOnlyAppTests() throws Exception {
FakeProjectFilesystem projectFilesystem = new FakeProjectFilesystem();
Expand Down Expand Up @@ -234,9 +289,11 @@ public void xctoolCommandWhichReturnsExitCode1DoesNotFailStep() throws Exception
.setDirectory(projectFilesystem.getRootPath().toAbsolutePath().toFile())
.setRedirectOutput(ProcessBuilder.Redirect.PIPE)
.build();
FakeProcess fakeXctoolSuccess = new FakeProcess(1, "", "");
// Test failure is indicated by xctool exiting with exit code 1, so it shouldn't
// fail the step.
FakeProcess fakeXctoolTestFailure = new FakeProcess(1, "", "");
FakeProcessExecutor processExecutor = new FakeProcessExecutor(
ImmutableMap.of(xctoolParams, fakeXctoolSuccess));
ImmutableMap.of(xctoolParams, fakeXctoolTestFailure));
ExecutionContext executionContext = TestExecutionContext.newBuilder()
.setProcessExecutor(processExecutor)
.setEnvironment(ImmutableMap.<String, String>of())
Expand Down Expand Up @@ -282,9 +339,9 @@ public void xctoolCommandWhichReturnsExitCode400FailsStep() throws Exception {
.setDirectory(projectFilesystem.getRootPath().toAbsolutePath().toFile())
.setRedirectOutput(ProcessBuilder.Redirect.PIPE)
.build();
FakeProcess fakeXctoolSuccess = new FakeProcess(400, "", "");
FakeProcess fakeXctoolFailure = new FakeProcess(400, "", "");
FakeProcessExecutor processExecutor = new FakeProcessExecutor(
ImmutableMap.of(xctoolParams, fakeXctoolSuccess));
ImmutableMap.of(xctoolParams, fakeXctoolFailure));
ExecutionContext executionContext = TestExecutionContext.newBuilder()
.setProcessExecutor(processExecutor)
.setEnvironment(ImmutableMap.<String, String>of())
Expand Down Expand Up @@ -383,6 +440,68 @@ public void xctoolCommandWithTestSelectorFiltersTests() throws Exception {
}
}

@Test
public void xctoolCommandWithTestSelectorFailsIfListTestsOnlyFails() throws Exception {
FakeProjectFilesystem projectFilesystem = new FakeProjectFilesystem();
XctoolRunTestsStep step = new XctoolRunTestsStep(
projectFilesystem,
Paths.get("/path/to/xctool"),
Optional.<Long>absent(),
"iphonesimulator",
Optional.<String>absent(),
ImmutableSet.of(Paths.get("/path/to/FooTest.xctest"), Paths.get("/path/to/BarTest.xctest")),
ImmutableMap.<Path, Path>of(),
Paths.get("/path/to/output.json"),
Optional.<XctoolRunTestsStep.StdoutReadingCallback>absent(),
Suppliers.ofInstance(Optional.of(Paths.get("/path/to/developer/dir"))),
TestSelectorList.builder()
.addRawSelectors("#.*Magic.*")
.build(),
Optional.<String>absent(),
Optional.<Path>absent(),
Optional.<String>absent(),
Optional.<String>absent());

ProcessExecutorParams xctoolListOnlyParams =
ProcessExecutorParams.builder()
.setCommand(
ImmutableList.of(
"/path/to/xctool",
"-reporter",
"json-stream",
"-sdk",
"iphonesimulator",
"run-tests",
"-logicTest",
"/path/to/FooTest.xctest",
"-logicTest",
"/path/to/BarTest.xctest",
"-listTestsOnly"))
.setEnvironment(ImmutableMap.of("DEVELOPER_DIR", "/path/to/developer/dir"))
.setDirectory(projectFilesystem.getRootPath().toAbsolutePath().toFile())
.setRedirectOutput(ProcessBuilder.Redirect.PIPE)
.build();
FakeProcess fakeXctoolListTestsFailureProcess =
new FakeProcess(42, "", "Chopper Dave, we have Uh-Oh");
FakeProcessExecutor processExecutor = new FakeProcessExecutor(
ImmutableMap.of(
xctoolListOnlyParams, fakeXctoolListTestsFailureProcess));
TestConsole testConsole = new TestConsole();
ExecutionContext executionContext = TestExecutionContext.newBuilder()
.setProcessExecutor(processExecutor)
.setEnvironment(ImmutableMap.<String, String>of())
.setConsole(testConsole)
.build();
assertThat(
step.execute(executionContext),
equalTo(42));
assertThat(
testConsole.getTextWrittenToStdErr(),
allOf(
containsString("xctool failed with exit code 42: Chopper Dave, we have Uh-Oh"),
containsString("Failed to query tests with xctool")));
}

@Test
public void xctoolCommandWithTestSelectorMatchingNothingDoesNotFail() throws Exception {
FakeProjectFilesystem projectFilesystem = new FakeProjectFilesystem();
Expand Down

0 comments on commit 3c42b08

Please sign in to comment.