On Thu, 4 Apr 2024 17:16:22 GMT, Mahendra Chhipa <mchh...@openjdk.org> wrote:
>> Updated following tests to use ProcessTools methods: >> test/jdk/java/lang/Thread/UncaughtExceptionsTest.java >> test/jdk/java/lang/annotation/LoaderLeakTest.java >> test/jdk/java/rmi/reliability/benchmark/bench/rmi/Main.java >> test/jdk/java/time/nontestng/java/time/chrono/HijrahConfigTest.java >> test/jdk/javax/naming/spi/providers/InitialContextTest.java > > Mahendra Chhipa has updated the pull request incrementally with one > additional commit since the last revision: > > Implemented review comments. > Updated EscapePath test. test/jdk/javax/naming/spi/providers/InitialContextTest.java line 268: > 266: OutputAnalyzer outputAnalyzer = > ProcessTools.executeCommand(commands); > 267: if(outputAnalyzer.getExitValue() != 0) { > 268: throw new RuntimeException(outputAnalyzer.getOutput()); I think it would be better to just call: outputAnalyzer.shouldHaveExitValue(0); that way it even prints the stdout/stderr logs and throws the exception. test/jdk/javax/naming/spi/providers/InitialContextTest.java line 286: > 284: OutputAnalyzer outputAnalyzer = > ProcessTools.executeCommand(commands.toArray(new String[commands.size()])); > 285: if(outputAnalyzer.getExitValue() != 0) { > 286: throw new RuntimeException(outputAnalyzer.getOutput()); Same comment as above `outputAnalyzer.shouldHaveExitValue(0)` would be better. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18602#discussion_r1559229610 PR Review Comment: https://git.openjdk.org/jdk/pull/18602#discussion_r1559230010