RFR: 8307778: com/sun/jdi/cds tests are not compatible with jtreg test factory

Chris Plummer cjplummer at openjdk.org
Wed May 15 19:19:00 UTC 2024


On Wed, 15 May 2024 05:59:56 GMT, Jaikiran Pai <jpai at 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.

Changes look good. I think the CR needs a better title. Something that mentions "Virtual". Also, it's not the "test factory" we area talking about here. It is the "test thread factory".

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

Marked as reviewed by cjplummer (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19244#pullrequestreview-2058820760


More information about the serviceability-dev mailing list