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 65:

> 63:  * @library /test/lib
> 64:  * @build jdk.test.lib.process.ProcessTools
> 65:  * @run main/othervm InitialContextTest

Hello Mahendra, I see that this test and one other test is being changed to 
`/othervm` and I suspect it's because, we now call:


System.setProperty("test.noclasspath", "true");

for the `ProcessTools` to skip adding the default `-cp` option to the launched 
Java process. 
In reality, we don't have to set that system property and thus you don't have 
to change this and the other test to `othervm`. See a previous discussion about 
the classpath handling by `ProcessTools` here 
https://github.com/openjdk/jdk/pull/17787/files/a9fcc6c2900356250b29b3d11b402790a84d9317#r1484276695
 - essentially, whatever classpath you end up passing as part of the command to 
`ProcessTools.createTestJavaProcessBuilder()` will end up getting used.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18602#discussion_r1559224415

Reply via email to