diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 40941d153..eccec59e8 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -31,6 +31,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/764[#764]: Fix IDEasy in CMD * 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 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 e9d1a8c71..f462bf800 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,14 +219,22 @@ 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)) { - return br.lines().toList(); + + String line; + while ((line = br.readLine()) != null) { + OutputMessage outputMessage = new OutputMessage(errorStream, line); + outputMessages.add(outputMessage); + } + + return null; } 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..6eaf01822 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,25 @@ 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 List outputMessages; + /** * @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.exitCode = ProcessResult.SUCCESS; this.directory = directory; this.now = LocalDateTime.now(); + this.outputMessages = new ArrayList(); } - /** - * @return the mocked {@link ProcessResult#getExitCode() exit code}. - */ - public int getExitCode() { - - return this.exitCode; - } - - /** - * @param exitCode the {@link #getExitCode() exit code}. - */ - public void setExitCode(int exitCode) { - - this.exitCode = exitCode; - } - - /** - * @return the {@link List} of mocked error messages. - */ - public List getErrors() { - - return errors; - } - - /** - * @return the {@link List} of mocked out messages. - */ - public List getOuts() { - - return outs; + public void addOutputMessage(OutputMessage message) { + this.outputMessages.add(message); } @Override @@ -121,6 +88,7 @@ public ProcessContext withPathEntry(Path path) { @Override public ProcessResult run(ProcessMode processMode) { + int exitCode = ProcessResult.SUCCESS; StringBuilder command = new StringBuilder("git"); for (String arg : this.arguments) { command.append(' '); @@ -131,7 +99,6 @@ public ProcessResult run(ProcessMode processMode) { if (this.arguments.contains("clean")) { try { Files.deleteIfExists(this.directory.resolve("new-folder")); - this.exitCode = 0; } catch (IOException e) { throw new RuntimeException(e); } @@ -139,8 +106,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"); - this.exitCode = 0; + OutputMessage outputMessage = new OutputMessage(false, "new-folder"); + this.outputMessages.add(outputMessage); } } if (this.arguments.contains("clone")) { @@ -149,14 +116,13 @@ public ProcessResult run(ProcessMode processMode) { Path newFile = Files.createFile(gitFolderPath.resolve("url")); // 3rd argument = repository Url Files.writeString(newFile, this.arguments.get(2)); - this.exitCode = 0; } catch (IOException e) { throw new RuntimeException(e); } } // always consider that files were changed if (this.arguments.contains("diff-index")) { - this.exitCode = 1; + exitCode = 1; } // changes file back to initial state (uses reference file in .git folder) if (this.arguments.contains("reset")) { @@ -165,7 +131,7 @@ public ProcessResult run(ProcessMode processMode) { Files.copy(gitFolderPath.resolve("objects").resolve("referenceFile"), this.directory.resolve("trackedFile"), StandardCopyOption.REPLACE_EXISTING); } - this.exitCode = 0; + exitCode = ProcessResult.SUCCESS; } catch (IOException e) { throw new RuntimeException(e); } @@ -175,13 +141,13 @@ public ProcessResult run(ProcessMode processMode) { Files.createDirectories(gitFolderPath); Path newFile = Files.createFile(gitFolderPath.resolve("update")); Files.writeString(newFile, this.now.toString()); - this.exitCode = 0; + exitCode = ProcessResult.SUCCESS; } catch (IOException e) { throw new RuntimeException(e); } } this.arguments.clear(); - return new ProcessResultImpl("git", command.toString(), this.exitCode, this.outs, this.errors); + return new ProcessResultImpl("git", command.toString(), exitCode, this.outputMessages); } } 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..e3fadb19f 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}. @@ -71,7 +72,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.addOutputMessage(outputMessage); // act context.getGitContext().pullOrClone(GitUrl.of(gitRepoUrl), tempDir); // assert @@ -89,7 +91,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.addOutputMessage(outputMessage); FileAccess fileAccess = new FileAccessImpl(context); Path gitFolderPath = tempDir.resolve(".git"); fileAccess.mkdirs(gitFolderPath); @@ -129,7 +132,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.addOutputMessage(outputMessage); // act context.getGitContext().pullOrCloneAndResetIfNeeded(new GitUrl(gitRepoUrl, "master"), tempDir, "origin"); // assert @@ -147,7 +151,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.addOutputMessage(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 100755 index 000000000..555c4265a --- /dev/null +++ b/cli/src/test/resources/process-context/log-order.sh @@ -0,0 +1,9 @@ +#!/bin/bash +echo "out1" +sleep 1 +echo "err1" >&2 +sleep 1 +echo "out2" +sleep 1 +echo "err2" >&2 +