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 271:
> 269: }
> 270: } catch (Exception ex) {
> 271: throw new RuntimeException(ex.getMessage());
This and other similar places in this PR will end up swallowing the underlying
exception. It would be better to just have the method have a `throws Exception`
or change this to:
throw new RuntimeException(ex);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18602#discussion_r1559235120
