Skip to content

Commit

Permalink
Don't buffer process output by default but do log output
Browse files Browse the repository at this point in the history
  • Loading branch information
@rchodava
rchodava committed Nov 3, 2016
1 parent af73ee0 commit 61504a4
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 40 deletions.
55 changes: 31 additions & 24 deletions core/src/main/java/foundation/stack/datamill/ProcessRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,37 +43,41 @@ public Thread newThread(Runnable r) {
}));

public static void runProcess(File workingDirectory, String... command) throws IOException {
runProcess(workingDirectory, false, command);
}

public static void runProcess(File workingDirectory, boolean returnOutput, String... command) throws IOException {
logger.debug("{}", Joiner.on(' ').join(command));

Process process = new ProcessBuilder().directory(workingDirectory).command(command).start();

try {
readLinesFromStream(process.getInputStream());
readLinesFromStream(process.getErrorStream());
readLinesFromStream(process.getInputStream(), returnOutput);
readLinesFromStream(process.getErrorStream(), returnOutput);
} catch (InterruptedException e) {
throw new IOException(e);
}
}

public static ExecutionResult runProcessAndWait(File workingDirectory, String... command) throws IOException {
return runProcessAndWait(workingDirectory, true, command);
return runProcessAndWait(workingDirectory, false, command);
}

public static ExecutionResult runProcessAndWait(File workingDirectory, boolean returnOutput, String... command) throws IOException {
public static ExecutionResult runProcessAndWait(File workingDirectory, boolean bufferOutput, String... command)
throws IOException {
logger.debug("{}", Joiner.on(' ').join(command));

Process process = new ProcessBuilder().directory(workingDirectory).command(command).start();

try {
ListenableFuture<List<String>> standardOutputFuture = null, standardErrorFuture = null;
if (returnOutput) {
standardOutputFuture = readLinesFromStream(process.getInputStream());
standardErrorFuture = readLinesFromStream(process.getErrorStream());
}
ListenableFuture<List<String>> standardOutputFuture, standardErrorFuture;

standardOutputFuture = readLinesFromStream(process.getInputStream(), bufferOutput);
standardErrorFuture = readLinesFromStream(process.getErrorStream(), bufferOutput);

int exitCode = process.waitFor();

if (returnOutput) {
if (bufferOutput) {
List<List<String>> results = Futures.allAsList(standardOutputFuture, standardErrorFuture).get(1, TimeUnit.SECONDS);
return new ExecutionResult(exitCode, results.size() > 0 ? results.get(0) : null, results.size() > 1 ? results.get(1) : null);
}
Expand All @@ -83,7 +87,8 @@ public static ExecutionResult runProcessAndWait(File workingDirectory, boolean r
}
}

private static ListenableFuture<List<String>> readLinesFromStream(InputStream inputStream) throws InterruptedException {
private static ListenableFuture<List<String>> readLinesFromStream(InputStream inputStream, boolean bufferOutput)
throws InterruptedException {
BufferedReader processOutput = new BufferedReader(new InputStreamReader(inputStream));
List<String> output = new CopyOnWriteArrayList<>();
return processStreamProcessors.submit(() -> {
Expand All @@ -92,7 +97,10 @@ private static ListenableFuture<List<String>> readLinesFromStream(InputStream in
do {
line = processOutput.readLine();
if (line != null) {
output.add(line);
if (bufferOutput) {
output.add(line);
}

logger.debug(line);
}
} while (line != null && !Thread.interrupted());
Expand All @@ -104,32 +112,31 @@ private static ListenableFuture<List<String>> readLinesFromStream(InputStream in

public static class ExecutionResult {
private final int exitCode;
private final List<String> standardOutput;
private final List<String> standardError;
private final List<String> bufferedStandardOutput;
private final List<String> bufferedStandardError;

public ExecutionResult(int exitCode, List<String> standardOutput, List<String> standardError) {
public ExecutionResult(int exitCode, List<String> bufferedStandardOutput, List<String> bufferedStandardError) {
this.exitCode = exitCode;
this.standardOutput = standardOutput;
this.standardError = standardError;
this.bufferedStandardOutput = bufferedStandardOutput;
this.bufferedStandardError = bufferedStandardError;
}

public ExecutionResult(int exitCode) {
this.exitCode = exitCode;
this.standardOutput = null;
this.standardError = null;
this.bufferedStandardOutput = null;
this.bufferedStandardError = null;
}

public int getExitCode() {
return exitCode;
}

public List<String> getStandardOutput() {
return standardOutput;
public List<String> getBufferedStandardOutput() {
return bufferedStandardOutput;
}

public List<String> getStandardError() {
return standardError;
public List<String> getBufferedStandardError() {
return bufferedStandardError;
}
}
}

26 changes: 13 additions & 13 deletions core/src/test/java/foundation/stack/datamill/ProcessRunnerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,31 +20,31 @@ public class ProcessRunnerTest {
@Test
public void runProcessAndWait_ReturnsExpectedResults_OnSuccessfulCommandExecution() throws IOException {
String[] command = new String[] {"java", "-version"};
ProcessRunner.ExecutionResult executionResult = ProcessRunner.runProcessAndWait(null, command);
ProcessRunner.ExecutionResult executionResult = ProcessRunner.runProcessAndWait(null, true, command);
assertEquals(executionResult.getExitCode(), 0);
// It is counter intuitive, but jdk uses standard error instead of standard output for showing version
assertEquals(3, executionResult.getStandardError().size());
assertTrue(executionResult.getStandardError().get(0).startsWith("java version "));
assertTrue(executionResult.getStandardOutput().isEmpty());
assertEquals(3, executionResult.getBufferedStandardError().size());
assertTrue(executionResult.getBufferedStandardError().get(0).startsWith("java version "));
assertTrue(executionResult.getBufferedStandardOutput().isEmpty());
}

@Test
public void runProcessAndWait_NoOutput_OnSuccessfulCommandExecution() throws IOException {
String[] command = new String[] {"java", "-version"};
ProcessRunner.ExecutionResult executionResult = ProcessRunner.runProcessAndWait(null, false, command);
assertEquals(executionResult.getExitCode(), 0);
assertNull(executionResult.getStandardError());
assertNull(executionResult.getStandardOutput());
assertNull(executionResult.getBufferedStandardError());
assertNull(executionResult.getBufferedStandardOutput());
}

@Test
public void runProcessAndWait_ReturnsExpectedResults_OnFailingCommandExecution() throws IOException {
Path tmpDir = Paths.get(System.getProperty("java.io.tmpdir"));
String[] command = new String[] {"git", "status"};
ProcessRunner.ExecutionResult executionResult = ProcessRunner.runProcessAndWait(tmpDir.toFile(), command);
ProcessRunner.ExecutionResult executionResult = ProcessRunner.runProcessAndWait(tmpDir.toFile(), true, command);
assertTrue(executionResult.getExitCode() != 0);
assertTrue(executionResult.getStandardOutput().isEmpty());
assertEquals("fatal: Not a git repository (or any of the parent directories): .git", executionResult.getStandardError().get(0));
assertTrue(executionResult.getBufferedStandardOutput().isEmpty());
assertEquals("fatal: Not a git repository (or any of the parent directories): .git", executionResult.getBufferedStandardError().get(0));
}

@Test
Expand All @@ -55,14 +55,14 @@ public void runProcess_PerformsAsExpected_OnSuccessfulCommandExecution() throws
boolean runningOnWindows = runningOnWindows();
String[] command = runningOnWindows ? new String[] {"dir", tempDir.toString()} : new String[] {"ls", "-la", tempDir.toString()};

ProcessRunner.ExecutionResult executionResult = ProcessRunner.runProcessAndWait(null, command);
ProcessRunner.ExecutionResult executionResult = ProcessRunner.runProcessAndWait(null, true, command);
assertEquals(executionResult.getExitCode(), 0);
assertTrue(executionResult.getStandardError().isEmpty());
assertTrue(executionResult.getBufferedStandardError().isEmpty());
if (runningOnWindows) {
assertTrue(executionResult.getStandardOutput().isEmpty());
assertTrue(executionResult.getBufferedStandardOutput().isEmpty());
}
else {
assertEquals("total 0", executionResult.getStandardOutput().get(0));
assertEquals("total 0", executionResult.getBufferedStandardOutput().get(0));
}
}

Expand Down
6 changes: 3 additions & 3 deletions cucumber/src/main/java/foundation/stack/datamill/cucumber/CommandLineSteps.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,11 @@ public void executeCommandFromRelativeLocation(String command, String relativePa
File workingDirectory = resolvedRelativePath != null ? new File(temporaryDirectory, resolvedRelativePath) : temporaryDirectory;
ProcessRunner.ExecutionResult executionResult = ProcessRunner.runProcessAndWait(workingDirectory, resolvedCommand.split(" "));
if (executionResult.getExitCode() != 0) {
logger.error("Error while executing {} with message {}", resolvedCommand, executionResult.getStandardError());
logger.error("Error while executing {} with message {}", resolvedCommand, executionResult.getBufferedStandardError());
fail("Received result code " + executionResult.getExitCode() + " after executing " + resolvedCommand);
}
if (executionResult.getStandardOutput() != null && !executionResult.getStandardOutput().isEmpty()) {
String commandResult = Joiner.on(System.getProperty("line.separator")).join(executionResult.getStandardOutput());
if (executionResult.getBufferedStandardOutput() != null && !executionResult.getBufferedStandardOutput().isEmpty()) {
String commandResult = Joiner.on(System.getProperty("line.separator")).join(executionResult.getBufferedStandardOutput());
logger.debug("Command execution of [{}] returned following results: {}", resolvedCommand, commandResult);
propertyStore.put(COMMAND_RESULT, commandResult);
}
Expand Down

0 comments on commit 61504a4

Please sign in to comment.