On Wed, 15 May 2024 05:59:56 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this change which removes the > `test/jdk/com/sun/jdi/cds/` tests from being problem listed when jtreg > launches these tests through a virtual thread? > > These tests aren't actually incompatible with virtual threads. The real issue > is that in https://bugs.openjdk.org/browse/JDK-8305608, a test infrastructure > class `test/jdk/com/sun/jdi/VMConnection.java` was changed to use the > `test.class.path` system property instead of the previously used > `test.classes`. > > That change missed out updating the `test/jdk/com/sun/jdi/cds/` tests to pass > along the classpath through the `test.class.path` system property. As a > result these tests still use the old `test.classes` system property to pass > around the test classpath and thus causes these tests to fail. The reason why > this only impacts virtual threads is noted by Chris in this JBS comment > https://bugs.openjdk.org/browse/JDK-8305608?focusedId=14572100&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14572100. > > The commit in this PR updates the `CDSJDITest` to pass along > `test.class.path` instead of `test.classes`. The `CDSJDITest` is only used in > the tests under `test/jdk/com/sun/jdi/cds/`, so nothing else should be > impacted by this change. > > I ran these changes against both launching with platform thread as well as > virtual thread and these tests now pass in both these cases. Thank you all for the reviews. > I think the CR needs a better title. Something that mentions "Virtual". Also, > it's not the "test factory" we are talking about here. It is the "test thread > factory". I have now updated the title of the JBS issue (and this PR) to "com/sun/jdi/cds tests fail with jtreg's Virtual test thread factory". No other changes have been done. I'll integrate this in a few hours from now. ------------- PR Comment: https://git.openjdk.org/jdk/pull/19244#issuecomment-2113729065
