RFR: JDK-8319678: Several tests from corelibs areas ignore VM flags [v2]

[Jaikiran Pai]

On Thu, 4 Apr 2024 17:16:22 GMT, Mahendra Chhipa <mchhipa at 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