diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 46d912799..04d00547d 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -19,6 +19,7 @@ Then run the `setup` and all should work fine. Release with new features and bugfixes: +* https://github.com/devonfw/IDEasy/issues/734[#734]: Improve ProcessResult: get out and err in order * https://github.com/devonfw/IDEasy/issues/774[#774]: HTTP proxy support not working properly * https://github.com/devonfw/IDEasy/issues/792[#792]: Honor new variable IDE_OPTIONS in ide command wrapper * https://github.com/devonfw/IDEasy/issues/589[#589]: Fix NLS Bundles for Linux and MacOS diff --git a/cli/src/main/java/com/devonfw/tools/ide/process/OutputMessage.java b/cli/src/main/java/com/devonfw/tools/ide/process/OutputMessage.java new file mode 100644 index 000000000..022baf69b --- /dev/null +++ b/cli/src/main/java/com/devonfw/tools/ide/process/OutputMessage.java @@ -0,0 +1,11 @@ +package com.devonfw.tools.ide.process; + +/** + * Represent an output message that can be either from stdout or stderr. + * + * @param error A boolean flag that indicates whether the output message is from stdout or stderr + * @param message A string containing the outout message + */ +public record OutputMessage(boolean error, String message) { + +} diff --git a/cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java b/cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java index 3290b8cf0..6b31213de 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java @@ -13,6 +13,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.stream.Collectors; import com.devonfw.tools.ide.cli.CliProcessException; @@ -168,17 +169,16 @@ public ProcessResult run(ProcessMode processMode) { this.processBuilder.command(args); - List out = null; - List err = null; + ConcurrentLinkedQueue output = new ConcurrentLinkedQueue<>(); Process process = this.processBuilder.start(); try { if (processMode == ProcessMode.DEFAULT_CAPTURE) { - CompletableFuture> outFut = readInputStream(process.getInputStream()); - CompletableFuture> errFut = readInputStream(process.getErrorStream()); - out = outFut.get(); - err = errFut.get(); + CompletableFuture> outFut = readInputStream(process.getInputStream(), false, output); + CompletableFuture> errFut = readInputStream(process.getErrorStream(), true, output); + outFut.get(); + errFut.get(); } int exitCode; @@ -189,7 +189,8 @@ public ProcessResult run(ProcessMode processMode) { exitCode = process.waitFor(); } - ProcessResult result = new ProcessResultImpl(this.executable.getFileName().toString(), command, exitCode, out, err); + List finalOutput = new ArrayList<>(output); + ProcessResult result = new ProcessResultImpl(this.executable.getFileName().toString(), command, exitCode, finalOutput); performLogging(result, exitCode, interpreter); @@ -218,13 +219,21 @@ public ProcessResult run(ProcessMode processMode) { * "https://stackoverflow.com/questions/14165517/processbuilder-forwarding-stdout-and-stderr-of-started-processes-without-blocki/57483714#57483714">StackOverflow * * @param is {@link InputStream}. + * @param errorStream to identify if the output came from stdout or stderr * @return {@link CompletableFuture}. */ - private static CompletableFuture> readInputStream(InputStream is) { + private static CompletableFuture> readInputStream(InputStream is, boolean errorStream, ConcurrentLinkedQueue outputMessages) { return CompletableFuture.supplyAsync(() -> { try (InputStreamReader isr = new InputStreamReader(is); BufferedReader br = new BufferedReader(isr)) { + + String line; + while ((line = br.readLine()) != null) { + OutputMessage outputMessage = new OutputMessage(errorStream, line); + outputMessages.add(outputMessage); + } + return br.lines().toList(); } catch (Throwable e) { throw new RuntimeException("There was a problem while executing the program", e); diff --git a/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResult.java b/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResult.java index 85a6d6bbb..e97348b77 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResult.java +++ b/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResult.java @@ -76,6 +76,12 @@ default boolean isSuccessful() { */ List getErr(); + /** + * @return the {@link List} with {@link OutputMessage} that captured on standard out and standard error lines. Will be {@code null} if not captured but + * redirected. + */ + List getOutputMessages(); + /** * Logs output and error messages on the provided log level. * diff --git a/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java b/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java index 4f09e9b51..fbb489667 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java +++ b/cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java @@ -19,9 +19,7 @@ public class ProcessResultImpl implements ProcessResult { private final int exitCode; - private final List out; - - private final List err; + private final List outputMessages; /** * The constructor. @@ -29,17 +27,15 @@ public class ProcessResultImpl implements ProcessResult { * @param executable the {@link #getExecutable() executable}. * @param command the {@link #getCommand() command}. * @param exitCode the {@link #getExitCode() exit code}. - * @param out the {@link #getOut() out}. - * @param err the {@link #getErr() err}. + * @param outputMessages {@link #getOutputMessages() output Messages}. */ - public ProcessResultImpl(String executable, String command, int exitCode, List out, List err) { + public ProcessResultImpl(String executable, String command, int exitCode, List outputMessages) { super(); this.executable = executable; this.command = command; this.exitCode = exitCode; - this.out = Objects.requireNonNullElse(out, Collections.emptyList()); - this.err = Objects.requireNonNullElse(err, Collections.emptyList()); + this.outputMessages = Objects.requireNonNullElse(outputMessages, Collections.emptyList()); } @Override @@ -63,13 +59,20 @@ public int getExitCode() { @Override public List getOut() { - return this.out; + return this.outputMessages.stream().filter(outputMessage -> !outputMessage.error()).map(OutputMessage::message).toList(); } @Override public List getErr() { - return this.err; + return this.outputMessages.stream().filter(OutputMessage::error).map(OutputMessage::message).toList(); + } + + @Override + public List getOutputMessages() { + + return this.outputMessages; + } @Override @@ -80,22 +83,23 @@ public void log(IdeLogLevel level, IdeContext context) { @Override public void log(IdeLogLevel outLevel, IdeContext context, IdeLogLevel errorLevel) { - if (!this.out.isEmpty()) { - doLog(outLevel, this.out, context); - } - if (!this.err.isEmpty()) { - doLog(errorLevel, this.err, context); + if (!this.outputMessages.isEmpty()) { + for (OutputMessage outputMessage : this.outputMessages) { + if (outputMessage.error()) { + doLog(errorLevel, outputMessage.message(), context); + } else { + doLog(outLevel, outputMessage.message(), context); + } + } } } - private void doLog(IdeLogLevel level, List lines, IdeContext context) { - for (String line : lines) { - // remove !MESSAGE from log message - if (line.startsWith("!MESSAGE ")) { - line = line.substring(9); - } - context.level(level).log(line); + private void doLog(IdeLogLevel level, String message, IdeContext context) { + // remove !MESSAGE from log message + if (message.startsWith("!MESSAGE ")) { + message = message.substring(9); } + context.level(level).log(message); } @Override diff --git a/cli/src/test/java/com/devonfw/tools/ide/context/ProcessContextGitMock.java b/cli/src/test/java/com/devonfw/tools/ide/context/ProcessContextGitMock.java index 5c891d257..405e834cf 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/context/ProcessContextGitMock.java +++ b/cli/src/test/java/com/devonfw/tools/ide/context/ProcessContextGitMock.java @@ -8,6 +8,7 @@ import java.util.ArrayList; import java.util.List; +import com.devonfw.tools.ide.process.OutputMessage; import com.devonfw.tools.ide.process.ProcessContext; import com.devonfw.tools.ide.process.ProcessErrorHandling; import com.devonfw.tools.ide.process.ProcessMode; @@ -21,59 +22,58 @@ public class ProcessContextGitMock implements ProcessContext { private final List arguments; - private final List errors; - - private final List outs; - private final LocalDateTime now; private int exitCode; private final Path directory; + private ProcessResult processResult; + /** * @param directory the {@link Path} to the git repository. */ public ProcessContextGitMock(Path directory) { this.arguments = new ArrayList<>(); - this.errors = new ArrayList<>(); - this.outs = new ArrayList<>(); + this.processResult = new ProcessResultImpl("git", "", 0, new ArrayList<>()); this.exitCode = ProcessResult.SUCCESS; this.directory = directory; this.now = LocalDateTime.now(); } /** - * @return the mocked {@link ProcessResult#getExitCode() exit code}. + * @return the mocked {@link ProcessResult} */ - public int getExitCode() { + public ProcessResult getProcessResult() { - return this.exitCode; + return this.processResult; } /** * @param exitCode the {@link #getExitCode() exit code}. + * @param output the list of {@link OutputMessage}} + * @return the mocked {@link ProcessResult} */ - public void setExitCode(int exitCode) { + public void setProcessResult(int exitCode, List output) { - this.exitCode = exitCode; + this.processResult = new ProcessResultImpl("git", "", exitCode, output); } /** - * @return the {@link List} of mocked error messages. + * @return the mocked {@link ProcessResult#getExitCode() exit code}. */ - public List getErrors() { + public int getExitCode() { - return errors; + return this.exitCode; } /** - * @return the {@link List} of mocked out messages. + * @param exitCode the {@link #getExitCode() exit code}. */ - public List getOuts() { + public void setExitCode(int exitCode) { - return outs; + this.exitCode = exitCode; } @Override @@ -139,7 +139,8 @@ public ProcessResult run(ProcessMode processMode) { // part of git cleanup checks if a new directory 'new-folder' exists if (this.arguments.contains("ls-files")) { if (Files.exists(this.directory.resolve("new-folder"))) { - this.outs.add("new-folder"); + OutputMessage outputMessage = new OutputMessage(false, "new-folder"); + processResult.getOutputMessages().add(outputMessage); this.exitCode = 0; } } @@ -181,7 +182,8 @@ public ProcessResult run(ProcessMode processMode) { } } this.arguments.clear(); - return new ProcessResultImpl("git", command.toString(), this.exitCode, this.outs, this.errors); + setProcessResult(getExitCode(), this.processResult.getOutputMessages()); + return this.processResult; } } diff --git a/cli/src/test/java/com/devonfw/tools/ide/git/GitContextTest.java b/cli/src/test/java/com/devonfw/tools/ide/git/GitContextTest.java index 9d56664a1..2ab98427d 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/git/GitContextTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/git/GitContextTest.java @@ -19,6 +19,7 @@ import com.devonfw.tools.ide.context.ProcessContextGitMock; import com.devonfw.tools.ide.io.FileAccess; import com.devonfw.tools.ide.io.FileAccessImpl; +import com.devonfw.tools.ide.process.OutputMessage; /** * Test of {@link GitContext}. @@ -34,6 +35,8 @@ private IdeTestContext newGitContext(Path dir) { this.processContext = new ProcessContextGitMock(dir); context.setProcessContext(processContext); context.setGitContext(new GitContextImpl(context)); + // reset ProcessResult instance + this.processContext.setProcessResult(0, new ArrayList<>()); return context; } @@ -71,7 +74,8 @@ public void testRunGitClone(@TempDir Path tempDir) { // arrange String gitRepoUrl = "https://github.com/test"; IdeTestContext context = newGitContext(tempDir); - this.processContext.getOuts().add("test-remote"); + OutputMessage outputMessage = new OutputMessage(false, "test-remote"); + this.processContext.getProcessResult().getOutputMessages().add(outputMessage); // act context.getGitContext().pullOrClone(GitUrl.of(gitRepoUrl), tempDir); // assert @@ -89,7 +93,8 @@ public void testRunGitPullWithoutForce(@TempDir Path tempDir) { // arrange String gitRepoUrl = "https://github.com/test"; IdeTestContext context = newGitContext(tempDir); - this.processContext.getOuts().add("test-remote"); + OutputMessage outputMessage = new OutputMessage(false, "test-remote"); + this.processContext.getProcessResult().getOutputMessages().add(outputMessage); FileAccess fileAccess = new FileAccessImpl(context); Path gitFolderPath = tempDir.resolve(".git"); fileAccess.mkdirs(gitFolderPath); @@ -129,7 +134,8 @@ public void testRunGitPullWithForceStartsReset(@TempDir Path tempDir) { throw new RuntimeException(e); } IdeTestContext context = newGitContext(tempDir); - this.processContext.getOuts().add("test-remote"); + OutputMessage outputMessage = new OutputMessage(false, "test-remote"); + this.processContext.getProcessResult().getOutputMessages().add(outputMessage); // act context.getGitContext().pullOrCloneAndResetIfNeeded(new GitUrl(gitRepoUrl, "master"), tempDir, "origin"); // assert @@ -147,7 +153,8 @@ public void testRunGitPullWithForceStartsCleanup(@TempDir Path tempDir) { // arrange String gitRepoUrl = "https://github.com/test"; IdeTestContext context = newGitContext(tempDir); - this.processContext.getOuts().add("test-remote"); + OutputMessage outputMessage = new OutputMessage(false, "test-remote"); + this.processContext.getProcessResult().getOutputMessages().add(outputMessage); GitContext gitContext = context.getGitContext(); FileAccess fileAccess = context.getFileAccess(); Path gitFolderPath = tempDir.resolve(".git"); diff --git a/cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java b/cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java index cc44398b8..251c318b7 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java @@ -221,6 +221,18 @@ public void enablingCaptureShouldRedirectAndCaptureStreamsWithErrorsCorrectly() assertThat(context).log(IdeLogLevel.ERROR).hasMessageContaining("another error message to stderr"); } + @Test + public void defaultCaptureShouldCaptureStreamsWithCorrectOrder() { + // arrange + IdeTestContext context = newContext(PROJECT_BASIC, null, false); + + // act + context.newProcess().executable(TEST_RESOURCES.resolve("process-context").resolve("log-order.sh")).run(); + + // assert + assertThat(context).log(IdeLogLevel.INFO).hasEntries("out1", "err1", "out2", "err2"); + } + private IdeLogLevel convertToIdeLogLevel(ProcessErrorHandling processErrorHandling) { return switch (processErrorHandling) { diff --git a/cli/src/test/java/com/devonfw/tools/ide/tool/ide/IdeToolDummyCommandletTest.java b/cli/src/test/java/com/devonfw/tools/ide/tool/ide/IdeToolDummyCommandletTest.java index 1bf582688..48beb6c99 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/tool/ide/IdeToolDummyCommandletTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/tool/ide/IdeToolDummyCommandletTest.java @@ -75,7 +75,7 @@ public ProcessResult runTool(ProcessMode processMode, GenericVersionRange toolVe // skip installation but trigger postInstall to test mocked plugin installation postInstall(true); - return new ProcessResultImpl(this.tool, this.tool, 0, List.of(), List.of()); + return new ProcessResultImpl(this.tool, this.tool, 0, List.of()); } @Override diff --git a/cli/src/test/resources/process-context/log-order.sh b/cli/src/test/resources/process-context/log-order.sh new file mode 100644 index 000000000..25ab60f7b --- /dev/null +++ b/cli/src/test/resources/process-context/log-order.sh @@ -0,0 +1,6 @@ +#!/bin/bash +echo "out1" +echo "err1" >&2 +sleep 5 +echo "out2" +echo "err2" >&2