From 22e8a97a1ce4e1c781fbc6f1e271c477fe95f069 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Fri, 18 Apr 2025 12:12:52 +0000 Subject: [PATCH] 8354988: Separate stderr and stdout in Executor class from jpackage test lib Reviewed-by: almatvee --- .../jdk/jpackage/test/ExecutorTest.java | 29 +- .../jdk/jpackage/test/PackageTestTest.java | 2 +- .../helpers/jdk/jpackage/test/Executor.java | 668 ++++++++++++++---- .../jdk/jpackage/test/JPackageCommand.java | 17 + .../jdk/jpackage/test/WindowsHelper.java | 13 +- test/jdk/tools/jpackage/share/BasicTest.java | 7 +- .../jpackage/windows/Win8301247Test.java | 36 +- 7 files changed, 601 insertions(+), 171 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/ExecutorTest.java b/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/ExecutorTest.java index 8175f49a2b2..c5d8aed845c 100644 --- a/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/ExecutorTest.java +++ b/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/ExecutorTest.java @@ -128,8 +128,8 @@ public class ExecutorTest extends JUnitAdapter { DUMP(Executor::dumpOutput), SAVE_ALL(Executor::saveOutput), SAVE_FIRST_LINE(Executor::saveFirstLineOfOutput), - DISCARD_STDOUT(NOP), - DISCARD_STDERR(NOP), + DISCARD_STDOUT(Executor::discardStdout), + DISCARD_STDERR(Executor::discardStderr), ; OutputControl(Consumer configureExector) { @@ -213,6 +213,9 @@ public class ExecutorTest extends JUnitAdapter { assertEquals(expectedCapturedSystemOut(commandWithDiscardedStreams), outputCapture.outLines()); assertEquals(expectedCapturedSystemErr(commandWithDiscardedStreams), outputCapture.errLines()); + assertEquals(expectedResultStdout(commandWithDiscardedStreams), result[0].stdout().getOutput()); + assertEquals(expectedResultStderr(commandWithDiscardedStreams), result[0].stderr().getOutput()); + if (!saveOutput()) { assertNull(result[0].getOutput()); } else { @@ -272,6 +275,28 @@ public class ExecutorTest extends JUnitAdapter { } } + private List expectedResultStdout(Command command) { + return expectedResultStream(command.stdout()); + } + + private List expectedResultStderr(Command command) { + if (outputControl.contains(OutputControl.SAVE_FIRST_LINE) && !command.stdout().isEmpty()) { + return List.of(); + } + return expectedResultStream(command.stderr()); + } + + private List expectedResultStream(List commandOutput) { + Objects.requireNonNull(commandOutput); + if (outputControl.contains(OutputControl.SAVE_ALL)) { + return commandOutput; + } else if (outputControl.contains(OutputControl.SAVE_FIRST_LINE)) { + return commandOutput.stream().findFirst().map(List::of).orElseGet(List::of); + } else { + return null; + } + } + private Command discardStreams(Command command) { return new Command(discardStdout() ? List.of() : command.stdout(), discardStderr() ? List.of() : command.stderr()); } diff --git a/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java b/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java index 3e14022a70e..da94db30925 100644 --- a/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java +++ b/test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java @@ -370,7 +370,7 @@ public class PackageTestTest extends JUnitAdapter { } catch (IOException ex) { throw new UncheckedIOException(ex); } - return new Executor.Result(actualJPackageExitCode, null, + return new Executor.Result(actualJPackageExitCode, this::getPrintableCommandLine).assertExitCodeIs(expectedExitCode); } }; diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java index 053674960c4..91625603a2b 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java @@ -22,9 +22,12 @@ */ package jdk.jpackage.test; +import static java.util.stream.Collectors.joining; + import java.io.BufferedReader; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; import java.io.PrintStream; @@ -32,7 +35,6 @@ import java.io.StringReader; import java.io.Writer; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -63,7 +65,7 @@ public final class Executor extends CommandArguments { } public Executor() { - saveOutputType = new HashSet<>(Set.of(SaveOutputType.NONE)); + outputStreamsControl = new OutputStreamsControl(); winEnglishOutput = false; } @@ -131,54 +133,48 @@ public final class Executor extends CommandArguments { } /** - * Configures this instance to save full output that command will produce. - * This function is mutual exclusive with - * saveFirstLineOfOutput() function. + * Configures this instance to save all stdout and stderr streams from the to be + * executed command. + *

+ * This function is mutually exclusive with {@link #saveFirstLineOfOutput()}. * * @return this */ public Executor saveOutput() { - saveOutputType.remove(SaveOutputType.FIRST_LINE); - saveOutputType.add(SaveOutputType.FULL); - return this; + return saveOutput(true); } /** - * Configures how to save output that command will produce. If - * v is true, the function call is equivalent to - * saveOutput() call. If v is false, - * the function will result in not preserving command output. + * Configures if all stdout and stderr streams from the to be executed command + * should be saved. + *

+ * If v is true, the function call is equivalent to + * {@link #saveOutput()} call. If v is false, command + * output will not be saved. + * + * @parameter v if both stdout and stderr streams should be saved * * @return this */ public Executor saveOutput(boolean v) { - if (v) { - saveOutput(); - } else { - saveOutputType.remove(SaveOutputType.FIRST_LINE); - saveOutputType.remove(SaveOutputType.FULL); - } - return this; + return setOutputControl(v, OutputControlOption.SAVE_ALL); } /** - * Configures this instance to save only the first line out output that - * command will produce. This function is mutual exclusive with - * saveOutput() function. + * Configures this instance to save the first line of a stream merged from + * stdout and stderr streams from the to be executed command. + *

+ * This function is mutually exclusive with {@link #saveOutput()}. * * @return this */ public Executor saveFirstLineOfOutput() { - saveOutputType.add(SaveOutputType.FIRST_LINE); - saveOutputType.remove(SaveOutputType.FULL); - return this; + return setOutputControl(true, OutputControlOption.SAVE_FIRST_LINE); } /** - * Configures this instance to dump all output that command will produce to - * System.out and System.err. Can be used together with saveOutput() and - * saveFirstLineOfOutput() to save command output and also copy it in the - * default output streams. + * Configures this instance to dump both stdout and stderr streams from the to + * be executed command into {@link System.out}. * * @return this */ @@ -187,26 +183,60 @@ public final class Executor extends CommandArguments { } public Executor dumpOutput(boolean v) { - if (v) { - saveOutputType.add(SaveOutputType.DUMP); - } else { - saveOutputType.remove(SaveOutputType.DUMP); - } + return setOutputControl(v, OutputControlOption.DUMP); + } + + public Executor discardStdout(boolean v) { + outputStreamsControl.stdout().discard(v); return this; } - public record Result(int exitCode, List output, Supplier cmdline) { + public Executor discardStdout() { + return discardStdout(true); + } + public Executor discardStderr(boolean v) { + outputStreamsControl.stderr().discard(v); + return this; + } + + public Executor discardStderr() { + return discardStderr(true); + } + + public interface Output { + public List getOutput(); + + public default String getFirstLineOfOutput() { + return findFirstLineOfOutput().orElseThrow(); + } + + public default Optional findFirstLineOfOutput() { + return getOutput().stream().findFirst(); + } + } + + public record Result(int exitCode, CommandOutput output, Supplier cmdline) implements Output { public Result { + Objects.requireNonNull(output); Objects.requireNonNull(cmdline); } - public String getFirstLineOfOutput() { - return output.get(0); + public Result(int exitCode, Supplier cmdline) { + this(exitCode, CommandOutput.EMPTY, cmdline); } + @Override public List getOutput() { - return output; + return output.lines().orElse(null); + } + + public Output stdout() { + return createView(output.stdoutLines()); + } + + public Output stderr() { + return createView(output.stderrLines()); } public Result assertExitCodeIs(int expectedExitCode) { @@ -223,6 +253,15 @@ public final class Executor extends CommandArguments { public int getExitCode() { return exitCode; } + + private static Output createView(Optional> lines) { + return new Output() { + @Override + public List getOutput() { + return lines.orElse(null); + } + }; + } } public Result executeWithoutExitCodeCheck() { @@ -278,27 +317,42 @@ public final class Executor extends CommandArguments { private static final long serialVersionUID = 1L; } - /* - * Repeates command "max" times and waits for "wait" seconds between each - * execution until command returns expected error code. + /** + * Executes the configured command {@code max} at most times and waits for + * {@code wait} seconds between each execution until the command exits with + * {@code expectedCode} exit code. + * + * @param expectedExitCode the expected exit code of the command + * @param max the maximum times to execute the command + * @param wait number of seconds to wait between executions of the + * command */ - public Result executeAndRepeatUntilExitCode(int expectedCode, int max, int wait) { + public Result executeAndRepeatUntilExitCode(int expectedExitCode, int max, int wait) { try { return tryRunMultipleTimes(() -> { Result result = executeWithoutExitCodeCheck(); - if (result.getExitCode() != expectedCode) { + if (result.getExitCode() != expectedExitCode) { throw new BadResultException(result); } return result; - }, max, wait).assertExitCodeIs(expectedCode); + }, max, wait).assertExitCodeIs(expectedExitCode); } catch (BadResultException ex) { - return ex.getValue().assertExitCodeIs(expectedCode); + return ex.getValue().assertExitCodeIs(expectedExitCode); } } - /* - * Repeates a "task" "max" times and waits for "wait" seconds between each - * execution until the "task" returns without throwing an exception. + /** + * Calls {@code task.get()} at most {@code max} times and waits for {@code wait} + * seconds between each call until {@code task.get()} invocation returns without + * throwing {@link RuntimeException} exception. + *

+ * Returns the object returned by the first {@code task.get()} invocation that + * didn't throw an exception or rethrows the last exception if all of + * {@code max} attempts ended in exception being thrown. + * + * @param task the object of which to call {@link Supplier#get()} function + * @param max the maximum times to execute the command + * @param wait number of seconds to wait between executions of the */ public static T tryRunMultipleTimes(Supplier task, int max, int wait) { RuntimeException lastException = null; @@ -334,9 +388,10 @@ public final class Executor extends CommandArguments { return saveOutput().executeWithoutExitCodeCheck().getOutput(); } - private boolean withSavedOutput() { - return saveOutputType.contains(SaveOutputType.FULL) || saveOutputType.contains( - SaveOutputType.FIRST_LINE); + private Executor setOutputControl(boolean set, OutputControlOption v) { + outputStreamsControl.stdout().set(set, v); + outputStreamsControl.stderr().set(set, v); + return this; } private Path executablePath() { @@ -349,8 +404,8 @@ public final class Executor extends CommandArguments { // If relative path to executable is used it seems to be broken when // ProcessBuilder changes the directory. On Windows it changes the // directory first and on Linux it looks up for executable before - // changing the directory. So to stay of safe side, use absolute path - // to executable. + // changing the directory. Use absolute path to executable to play + // it safely on all platforms. return executable.toAbsolutePath(); } @@ -371,18 +426,14 @@ public final class Executor extends CommandArguments { if (winTmpDir != null) { builder.environment().put("TMP", winTmpDir); } + + outputStreamsControl.applyTo(builder); + StringBuilder sb = new StringBuilder(getPrintableCommandLine()); - if (withSavedOutput()) { - builder.redirectErrorStream(true); - sb.append("; save output"); - } else if (saveOutputType.contains(SaveOutputType.DUMP)) { - builder.inheritIO(); - sb.append("; inherit I/O"); - } else { - builder.redirectError(ProcessBuilder.Redirect.DISCARD); - builder.redirectOutput(ProcessBuilder.Redirect.DISCARD); - sb.append("; discard I/O"); - } + outputStreamsControl.describe().ifPresent(desc -> { + sb.append("; ").append(desc); + }); + if (directory != null) { builder.directory(directory.toFile()); sb.append(String.format("; in directory [%s]", directory)); @@ -414,98 +465,110 @@ public final class Executor extends CommandArguments { trace("Execute " + sb.toString() + "..."); Process process = builder.start(); - List outputLines = null; - if (withSavedOutput()) { - try (BufferedReader outReader = new BufferedReader( - new InputStreamReader(process.getInputStream()))) { - if (saveOutputType.contains(SaveOutputType.DUMP) - || saveOutputType.contains(SaveOutputType.FULL)) { - outputLines = outReader.lines().collect(Collectors.toList()); - } else { - outputLines = Optional.ofNullable(outReader.readLine()).map(List::of).orElseGet(List::of); - outReader.transferTo(Writer.nullWriter()); - } - } finally { - if (saveOutputType.contains(SaveOutputType.DUMP) && outputLines != null) { - outputLines.stream().forEach(System.out::println); - if (saveOutputType.contains(SaveOutputType.FIRST_LINE)) { - // Pick the first line of saved output if there is one - for (String line: outputLines) { - outputLines = List.of(line); - break; - } - } - } - } - } + final var output = combine( + processProcessStream(outputStreamsControl.stdout(), process.getInputStream()), + processProcessStream(outputStreamsControl.stderr(), process.getErrorStream())); final int exitCode = process.waitFor(); trace("Done. Exit code: " + exitCode); - final List output; - if (outputLines != null) { - output = Collections.unmodifiableList(outputLines); - } else { - output = null; - } return createResult(exitCode, output); } private int runToolProvider(PrintStream out, PrintStream err) { - trace("Execute " + getPrintableCommandLine() + "..."); + final var sb = new StringBuilder(getPrintableCommandLine()); + outputStreamsControl.describe().ifPresent(desc -> { + sb.append("; ").append(desc); + }); + trace("Execute " + sb + "..."); final int exitCode = toolProvider.run(out, err, args.toArray( String[]::new)); trace("Done. Exit code: " + exitCode); return exitCode; } - private Result createResult(int exitCode, List output) { - return new Result(exitCode, output, this::getPrintableCommandLine); + private Result runToolProvider() throws IOException { + final var toolProviderStreamConfig = ToolProviderStreamConfig.create(outputStreamsControl); + + final var exitCode = runToolProvider(toolProviderStreamConfig); + + final var output = combine( + read(outputStreamsControl.stdout(), toolProviderStreamConfig.out()), + read(outputStreamsControl.stderr(), toolProviderStreamConfig.err())); + return createResult(exitCode, output); } - private Result runToolProvider() throws IOException { - if (!withSavedOutput()) { - if (saveOutputType.contains(SaveOutputType.DUMP)) { - return createResult(runToolProvider(System.out, System.err), null); - } - - PrintStream nullPrintStream = new PrintStream(new OutputStream() { - @Override - public void write(int b) { - // Nop - } - }); - return createResult(runToolProvider(nullPrintStream, nullPrintStream), null); + private int runToolProvider(ToolProviderStreamConfig cfg) throws IOException { + try { + return runToolProvider(cfg.out().ps(), cfg.err().ps()); + } finally { + cfg.out().ps().flush(); + cfg.err().ps().flush(); } + } - try (ByteArrayOutputStream buf = new ByteArrayOutputStream(); - PrintStream ps = new PrintStream(buf)) { - final var exitCode = runToolProvider(ps, ps); - ps.flush(); - final List output; - final var bufAsString = buf.toString(); - try (BufferedReader bufReader = new BufferedReader(new StringReader( - bufAsString))) { - if (saveOutputType.contains(SaveOutputType.FIRST_LINE)) { - output = bufReader.lines().findFirst().map(List::of).orElseGet(List::of); - } else if (saveOutputType.contains(SaveOutputType.FULL)) { - output = bufReader.lines().collect(Collectors.toUnmodifiableList()); - } else { - output = null; - } - - if (saveOutputType.contains(SaveOutputType.DUMP)) { - Stream lines; - if (saveOutputType.contains(SaveOutputType.FULL)) { - lines = output.stream(); - } else { - lines = new BufferedReader(new StringReader(bufAsString)).lines(); - } - lines.forEach(System.out::println); + private static Optional> processProcessStream(OutputControl outputControl, InputStream in) throws IOException { + List outputLines = null; + try (final var bufReader = new BufferedReader(new InputStreamReader(in))) { + if (outputControl.dump() || outputControl.saveAll()) { + outputLines = bufReader.lines().toList(); + } else if (outputControl.saveFirstLine()) { + outputLines = Optional.ofNullable(bufReader.readLine()).map(List::of).orElseGet(List::of); + // Read all input, or the started process may exit with an error (cmd.exe does so). + bufReader.transferTo(Writer.nullWriter()); + } else { + // This should be empty input stream, fetch it anyway. + bufReader.transferTo(Writer.nullWriter()); + } + } finally { + if (outputControl.dump() && outputLines != null) { + outputLines.forEach(System.out::println); + if (outputControl.saveFirstLine()) { + outputLines = outputLines.stream().findFirst().map(List::of).orElseGet(List::of); } } - return createResult(exitCode, output); + if (!outputControl.save()) { + outputLines = null; + } } + return Optional.ofNullable(outputLines); + } + + private static Optional> read(OutputControl outputControl, CachingPrintStream cps) throws IOException { + final var bufferAsString = cps.bufferContents(); + try (final var bufReader = new BufferedReader(new StringReader(bufferAsString.orElse("")))) { + if (outputControl.saveFirstLine()) { + return Optional.of(bufReader.lines().findFirst().map(List::of).orElseGet(List::of)); + } else if (outputControl.saveAll()) { + return Optional.of(bufReader.lines().toList()); + } else if (bufferAsString.isPresent()) { + return Optional.of(List.of()); + } else { + return Optional.empty(); + } + } + } + + private CommandOutput combine(Optional> out, Optional> err) { + if (out.isEmpty() && err.isEmpty()) { + return new CommandOutput(); + } else if (out.isEmpty()) { + return new CommandOutput(err, -1); + } else if (err.isEmpty()) { + return new CommandOutput(out, Integer.MAX_VALUE); + } else { + final var combined = Stream.of(out, err).map(Optional::orElseThrow).flatMap(List::stream); + if (outputStreamsControl.stdout().saveFirstLine() && outputStreamsControl.stderr().saveFirstLine()) { + return new CommandOutput(Optional.of(combined.findFirst().map(List::of).orElseGet(List::of)), + Integer.min(1, out.orElseThrow().size())); + } else { + return new CommandOutput(Optional.of(combined.toList()), out.orElseThrow().size()); + } + } + } + + private Result createResult(int exitCode, CommandOutput output) { + return new Result(exitCode, output, this::getPrintableCommandLine); } public String getPrintableCommandLine() { @@ -539,16 +602,343 @@ public final class Executor extends CommandArguments { TKit.trace(String.format("exec: %s", msg)); } + private static PrintStream nullPrintStream() { + return new PrintStream(OutputStream.nullOutputStream()); + } + + private record OutputStreamsControl(OutputControl stdout, OutputControl stderr) { + OutputStreamsControl { + Objects.requireNonNull(stdout); + Objects.requireNonNull(stderr); + } + + OutputStreamsControl() { + this(new OutputControl(), new OutputControl()); + } + + void applyTo(ProcessBuilder pb) { + pb.redirectOutput(stdout.asProcessBuilderRedirect()); + pb.redirectError(stderr.asProcessBuilderRedirect()); + } + + Optional describe() { + final List tokens = new ArrayList<>(); + if (stdout.save() || stderr.save()) { + streamsLabel("save ", true).ifPresent(tokens::add); + } + if (stdout.dump() || stderr.dump()) { + streamsLabel("inherit ", true).ifPresent(tokens::add); + } + streamsLabel("discard ", false).ifPresent(tokens::add); + if (tokens.isEmpty()) { + return Optional.empty(); + } else { + return Optional.of(String.join("; ", tokens)); + } + } + + Optional streamsLabel(String prefix, boolean negate) { + Objects.requireNonNull(prefix); + final var str = Stream.of(stdoutLabel(negate), stderrLabel(negate)) + .filter(Optional::isPresent) + .map(Optional::orElseThrow) + .collect(joining("+")); + if (str.isEmpty()) { + return Optional.empty(); + } else { + return Optional.of(prefix + str); + } + } + + private Optional stdoutLabel(boolean negate) { + if ((stdout.discard() && !negate) || (!stdout.discard() && negate)) { + return Optional.of("out"); + } else { + return Optional.empty(); + } + } + + private Optional stderrLabel(boolean negate) { + if ((stderr.discard() && !negate) || (!stderr.discard() && negate)) { + return Optional.of("err"); + } else { + return Optional.empty(); + } + } + } + + private record CachingPrintStream(PrintStream ps, Optional buf) { + CachingPrintStream { + Objects.requireNonNull(ps); + Objects.requireNonNull(buf); + } + + Optional bufferContents() { + return buf.map(ByteArrayOutputStream::toString); + } + + static Builder build() { + return new Builder(); + } + + static final class Builder { + + Builder save(boolean v) { + save = v; + return this; + } + + Builder discard(boolean v) { + discard = v; + return this; + } + + Builder dumpStream(PrintStream v) { + dumpStream = v; + return this; + } + + CachingPrintStream create() { + final Optional buf; + if (save && !discard) { + buf = Optional.of(new ByteArrayOutputStream()); + } else { + buf = Optional.empty(); + } + + final PrintStream ps; + if (buf.isPresent() && dumpStream != null) { + ps = new PrintStream(new TeeOutputStream(List.of(buf.orElseThrow(), dumpStream)), true, dumpStream.charset()); + } else if (!discard) { + ps = buf.map(PrintStream::new).or(() -> Optional.ofNullable(dumpStream)).orElseGet(Executor::nullPrintStream); + } else { + ps = nullPrintStream(); + } + + return new CachingPrintStream(ps, buf); + } + + private boolean save; + private boolean discard; + private PrintStream dumpStream; + } + } + + private record ToolProviderStreamConfig(CachingPrintStream out, CachingPrintStream err) { + ToolProviderStreamConfig { + Objects.requireNonNull(out); + Objects.requireNonNull(err); + } + + static ToolProviderStreamConfig create(OutputStreamsControl cfg) { + final var errCfgBuilder = cfg.stderr().buildCachingPrintStream(System.err); + if (cfg.stderr().dump() && cfg.stderr().save()) { + errCfgBuilder.dumpStream(System.out); + } + return new ToolProviderStreamConfig( + cfg.stdout().buildCachingPrintStream(System.out).create(), errCfgBuilder.create()); + } + } + + private static final class OutputControl { + + boolean save() { + return save.isPresent(); + } + + boolean saveAll() { + return save.orElse(null) == OutputControlOption.SAVE_ALL; + } + + boolean saveFirstLine() { + return save.orElse(null) == OutputControlOption.SAVE_FIRST_LINE; + } + + boolean discard() { + return discard || (!dump && save.isEmpty()); + } + + boolean dump() { + return !discard && dump; + } + + OutputControl dump(boolean v) { + this.dump = v; + return this; + } + + OutputControl discard(boolean v) { + this.discard = v; + return this; + } + + OutputControl saveAll(boolean v) { + if (v) { + save = Optional.of(OutputControlOption.SAVE_ALL); + } else { + save = Optional.empty(); + } + return this; + } + + OutputControl saveFirstLine(boolean v) { + if (v) { + save = Optional.of(OutputControlOption.SAVE_FIRST_LINE); + } else { + save = Optional.empty(); + } + return this; + } + + OutputControl set(boolean set, OutputControlOption v) { + switch (v) { + case DUMP -> dump(set); + case SAVE_ALL -> saveAll(set); + case SAVE_FIRST_LINE -> saveFirstLine(set); + } + return this; + } + + ProcessBuilder.Redirect asProcessBuilderRedirect() { + if (discard()) { + return ProcessBuilder.Redirect.DISCARD; + } else if (dump && !save()) { + return ProcessBuilder.Redirect.INHERIT; + } else { + return ProcessBuilder.Redirect.PIPE; + } + } + + CachingPrintStream.Builder buildCachingPrintStream(PrintStream dumpStream) { + Objects.requireNonNull(dumpStream); + final var builder = CachingPrintStream.build().save(save()).discard(discard()); + if (dump()) { + builder.dumpStream(dumpStream); + } + return builder; + } + + private boolean dump; + private boolean discard; + private Optional save = Optional.empty(); + } + + private static final class TeeOutputStream extends OutputStream { + + public TeeOutputStream(Iterable streams) { + streams.forEach(Objects::requireNonNull); + this.streams = streams; + } + + @Override + public void write(int b) throws IOException { + for (final var out : streams) { + out.write(b); + } + } + + @Override + public void write(byte[] b) throws IOException { + for (final var out : streams) { + out.write(b); + } + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + for (final var out : streams) { + out.write(b, off, len); + } + } + + @Override + public void flush() throws IOException { + forEach(OutputStream::flush); + } + + @Override + public void close() throws IOException { + forEach(OutputStream::close); + } + + private void forEach(OutputStreamConsumer c) throws IOException { + IOException firstEx = null; + for (final var out : streams) { + try { + c.accept(out); + } catch (IOException e) { + if (firstEx == null) { + firstEx = e; + } + } + } + if (firstEx != null) { + throw firstEx; + } + } + + @FunctionalInterface + private static interface OutputStreamConsumer { + void accept(OutputStream out) throws IOException; + } + + private final Iterable streams; + } + + private static final class CommandOutput { + CommandOutput(Optional> lines, int stdoutLineCount) { + this.lines = Objects.requireNonNull(lines); + this.stdoutLineCount = stdoutLineCount; + } + + CommandOutput() { + this(Optional.empty(), 0); + } + + Optional> lines() { + return lines; + } + + Optional> stdoutLines() { + if (lines.isEmpty() || stdoutLineCount < 0) { + return Optional.empty(); + } + + final var theLines = lines.orElseThrow(); + if (stdoutLineCount == theLines.size()) { + return lines; + } else { + return Optional.of(theLines.subList(0, Integer.min(stdoutLineCount, theLines.size()))); + } + } + + Optional> stderrLines() { + if (lines.isEmpty() || stdoutLineCount > lines.orElseThrow().size()) { + return Optional.empty(); + } else if (stdoutLineCount == 0) { + return lines; + } else { + final var theLines = lines.orElseThrow(); + return Optional.of(theLines.subList(stdoutLineCount, theLines.size())); + } + } + + private final Optional> lines; + private final int stdoutLineCount; + + static final CommandOutput EMPTY = new CommandOutput(); + } + private ToolProvider toolProvider; private Path executable; - private Set saveOutputType; + private OutputStreamsControl outputStreamsControl; private Path directory; private Set removeEnvVars = new HashSet<>(); private Map setEnvVars = new HashMap<>(); private boolean winEnglishOutput; private String winTmpDir = null; - private static enum SaveOutputType { - NONE, FULL, FIRST_LINE, DUMP - }; + private static enum OutputControlOption { + SAVE_ALL, SAVE_FIRST_LINE, DUMP + } } diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java index b4d2d1872eb..cb9e3a687c7 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java @@ -67,6 +67,8 @@ public class JPackageCommand extends CommandArguments { args.addAll(cmd.args); withToolProvider = cmd.withToolProvider; saveConsoleOutput = cmd.saveConsoleOutput; + discardStdout = cmd.discardStdout; + discardStderr = cmd.discardStderr; suppressOutput = cmd.suppressOutput; ignoreDefaultRuntime = cmd.ignoreDefaultRuntime; ignoreDefaultVerbose = cmd.ignoreDefaultVerbose; @@ -679,6 +681,18 @@ public class JPackageCommand extends CommandArguments { return this; } + public JPackageCommand discardStdout(boolean v) { + verifyMutable(); + discardStdout = v; + return this; + } + + public JPackageCommand discardStderr(boolean v) { + verifyMutable(); + discardStderr = v; + return this; + } + public JPackageCommand dumpOutput(boolean v) { verifyMutable(); suppressOutput = !v; @@ -770,6 +784,7 @@ public class JPackageCommand extends CommandArguments { private Executor createExecutor() { Executor exec = new Executor() .saveOutput(saveConsoleOutput).dumpOutput(!suppressOutput) + .discardStdout(discardStdout).discardStderr(discardStderr) .setDirectory(executeInDirectory) .addArguments(args); @@ -1272,6 +1287,8 @@ public class JPackageCommand extends CommandArguments { private Boolean withToolProvider; private boolean saveConsoleOutput; + private boolean discardStdout; + private boolean discardStderr; private boolean suppressOutput; private boolean ignoreDefaultRuntime; private boolean ignoreDefaultVerbose; diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java index bc67f6e417d..1c6ac9a7669 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java @@ -323,18 +323,19 @@ public class WindowsHelper { private static long[] findAppLauncherPIDs(JPackageCommand cmd, String launcherName) { // Get the list of PIDs and PPIDs of app launcher processes. Run setWinRunWithEnglishOutput(true) for JDK-8344275. // wmic process where (name = "foo.exe") get ProcessID,ParentProcessID - List output = Executor.of("wmic", "process", "where", "(name", + final var result = Executor.of("wmic", "process", "where", "(name", "=", "\"" + cmd.appLauncherPath(launcherName).getFileName().toString() + "\"", ")", "get", "ProcessID,ParentProcessID").dumpOutput(true).saveOutput(). - setWinRunWithEnglishOutput(true).executeAndGetOutput(); - if ("No Instance(s) Available.".equals(output.getFirst().trim())) { + setWinRunWithEnglishOutput(true).execute(); + if ("No Instance(s) Available.".equals(result.stderr().findFirstLineOfOutput().map(String::trim).orElse(""))) { return new long[0]; } - String[] headers = Stream.of(output.getFirst().split("\\s+", 2)).map( + final var stdout = result.stdout(); + String[] headers = Stream.of(stdout.getFirstLineOfOutput().split("\\s+", 2)).map( String::trim).map(String::toLowerCase).toArray(String[]::new); - Pattern pattern; + final Pattern pattern; if (headers[0].equals("parentprocessid") && headers[1].equals( "processid")) { pattern = Pattern.compile("^(?\\d+)\\s+(?\\d+)\\s+$"); @@ -346,7 +347,7 @@ public class WindowsHelper { "Unrecognizable output of \'wmic process\' command"); } - List processes = output.stream().skip(1).map(line -> { + List processes = stdout.getOutput().stream().skip(1).map(line -> { Matcher m = pattern.matcher(line); long[] pids = null; if (m.matches()) { diff --git a/test/jdk/tools/jpackage/share/BasicTest.java b/test/jdk/tools/jpackage/share/BasicTest.java index 68c1b2c76cb..bfcfe7587d4 100644 --- a/test/jdk/tools/jpackage/share/BasicTest.java +++ b/test/jdk/tools/jpackage/share/BasicTest.java @@ -237,20 +237,21 @@ public final class BasicTest { final var cmd = JPackageCommand.helloAppImage() .ignoreDefaultVerbose(true) .useToolProvider(false) + .discardStdout(true) .removeArgumentWithValue("--main-class"); if (verbose) { cmd.addArgument("--verbose"); } - final var textVerifier = Stream.of( + cmd.validateOutput(Stream.of( List.of("error.no-main-class-with-main-jar", "hello.jar"), List.of("error.no-main-class-with-main-jar.advice", "hello.jar") ).map(args -> { return JPackageStringBundle.MAIN.cannedFormattedString(args.getFirst(), args.subList(1, args.size()).toArray()); - }).map(CannedFormattedString::getValue).map(TKit::assertTextStream).reduce(TKit.TextStreamVerifier::andThen).orElseThrow(); + }).toArray(CannedFormattedString[]::new)); - textVerifier.apply(cmd.saveConsoleOutput(true).execute(1).getOutput().stream().filter(Predicate.not(JPackageCommand::withTimestamp))); + cmd.execute(1); } @Test diff --git a/test/jdk/tools/jpackage/windows/Win8301247Test.java b/test/jdk/tools/jpackage/windows/Win8301247Test.java index b0c11147a17..2f98141dcb4 100644 --- a/test/jdk/tools/jpackage/windows/Win8301247Test.java +++ b/test/jdk/tools/jpackage/windows/Win8301247Test.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -21,14 +21,12 @@ * questions. */ -import java.io.IOException; +import static jdk.jpackage.test.WindowsHelper.killAppLauncherProcess; + import java.time.Duration; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import jdk.jpackage.test.JPackageCommand; import jdk.jpackage.test.Annotations.Test; import jdk.jpackage.test.HelloApp; -import static jdk.jpackage.test.WindowsHelper.killAppLauncherProcess; +import jdk.jpackage.test.JPackageCommand; /** * Test that terminating of the parent app launcher process automatically @@ -48,7 +46,7 @@ import static jdk.jpackage.test.WindowsHelper.killAppLauncherProcess; public class Win8301247Test { @Test - public void test() throws IOException, InterruptedException { + public void test() throws InterruptedException { var cmd = JPackageCommand.helloAppImage().ignoreFakeRuntime(); // Launch the app in a way it doesn't exit to let us trap app laucnher @@ -56,22 +54,20 @@ public class Win8301247Test { cmd.addArguments("--java-options", "-Djpackage.test.noexit=true"); cmd.executeAndAssertImageCreated(); - try ( // Launch the app in a separate thread - ExecutorService exec = Executors.newSingleThreadExecutor()) { - exec.execute(() -> { - HelloApp.executeLauncher(cmd); - }); + // Launch the app in a separate thread + new Thread(() -> { + HelloApp.executeLauncher(cmd); + }).start(); - // Wait a bit to let the app start - Thread.sleep(Duration.ofSeconds(10)); + // Wait a bit to let the app start + Thread.sleep(Duration.ofSeconds(10)); - // Find the main app launcher process and kill it - killAppLauncherProcess(cmd, null, 2); + // Find the main app launcher process and kill it + killAppLauncherProcess(cmd, null, 2); - // Wait a bit and check if child app launcher process is still running (it must NOT) - Thread.sleep(Duration.ofSeconds(5)); + // Wait a bit and check if child app launcher process is still running (it must NOT) + Thread.sleep(Duration.ofSeconds(5)); - killAppLauncherProcess(cmd, null, 0); - } + killAppLauncherProcess(cmd, null, 0); } }