On Tue, 20 Oct 2026 06:30:46 GMT, David Holmes <[email protected]> wrote:
>> Can I please get a review of this test-only change which addresses the >> failure reported in https://bugs.openjdk.org/browse/JDK-8394341? >> >> This test launches processes as part of its test execution. It was recently >> refactored to use the commonly used JDK internal >> `jdk.test.lib.process.ProcessTools` test library. After this refactoring, >> if/when this test gets launched on a virtual thread (through the >> `-testThreadFactory:Virtual`), then the `testWithClassPathSetViaProperty()` >> test method fails because it launches a `java` processes with a >> custom/controlled classpath. >> >> Given the way `ProcessTools` library is implemented, when launching a `java` >> process in the presence of `-testThreadFactory:Virtual`, the launched >> process is expected to contain the `jdk.test.lib.process.ProcessTools` class >> in its classpath. Given that `testWithClassPathSetViaProperty()`, configures >> a classpath of its own, the test thus fails due to the missing >> `jdk.test.lib.process.ProcessTools`. >> >> In practice, this test shouldn't be run when launching jtreg with >> `-testThreadFactory:Virtual`, neither should the child processes be passed >> any additional arguments than what the test method itself configures. The >> commit in this PR sets up the relevant `@requires` in the test definition to >> prevent such executions. The test methods themselves has been updated to use >> `ProcessTools.executeCommand(...)` to make it clear that the child >> processes' arguments shouldn't be affected by how jtreg is launched (for >> example, by passing `-javaoptions:...`) >> >> >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > test/jdk/tools/launcher/MiscTests.java line 85: > >> 83: mainClass >> 84: ); >> 85: final OutputAnalyzer oa = launchJava(javaCmdArgs); > > Can't we just change to use ` executeLimitedTestJava` here? Hello David, the `ProcessTools.executeLimitedTestJava(...)` does additional things with the classpath: * <p>Unless the "test.noclasspath" property is "true" the * classpath property "java.class.path" is appended to the command * line and the environment of the ProcessBuilder is modified to * remove "CLASSPATH". So I didn't want it to interfere with this test in any manner. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33345#discussion_r4322139175
