On Thu, 30 Jun 2022 06:34:30 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Yes, it requires `-Dmain.wrapper=Virtual`, and that was intentional. My >> thinking was that we don't have any com/sun/jdi tests that do any virtual >> thread testing unless run with `-Dmain.wrapper=Virtual`, so why should this >> test be any different? If I made it do virtual thread testing without >> `-Dmain.wrapper=Virtual`, then the `-Dmain.wrapper=Virtual` testing becomes >> redundant. > > The test uses a platform thread by default so it's not testing the issue in > JDK-8287847. The suggestion is to have it re-run with the system property > set, like this: > > @run main/othervm SuspendAfterDeath > @run main/othervm -Dmain.wrapper=Virtual SuspendAfterDeath > > I accept this may be different to the existing com/sun/jdi tests but they > pre-date virtual threads. The @run suggestion doesn't work. There are two issues. First, it does not trigger jtreg to use TestScaffold.main(), which is what normally happens when you use TEST_VM_OPTS="-Dmain.wrapper=Virtual". So the end result is that the test is run with a platform thread. The other issue is that if the test run is done with TEST_VM_OPTS="-Dmain.wrapper=Virtual", then TestScaffold.main() cannot handle any arguments added to the @run other than the main class name. You end up with CNFE for the first argument that appears after "SuspendAfterDeath". The above issues aside, I still don't understand why we would want to do any virtual thread testing with com/sun/jdi tests when not using TEST_VM_OPTS="-Dmain.wrapper=Virtual". It's not how we've tested JDI support for virtual thread in the past. We've always relied on explicit virtual thread test runs using TEST_VM_OPTS="-Dmain.wrapper=Virtual". Same is true of the nsk jdi, jdb, and jdwp tests. If you really want this test handled differently because it was written to test a specific bug, then it probably does not belong in com/sun/jdi, which means it can't rely on TestScaffold. One other thing is that you don't really need -Dmain.wrapper=Virtual to make this test do the testing on a virtual thread. It can just be the default mode like you used to have when you first wrote it. However, I ran into problems with that when also using TEST_VM_OPTS="-Dmain.wrapper=Virtual" due to conflicts in argument setup between the test and TestScaffold. However, with the changes now in parseArgs() to check test.enable.preview, it might not be necessary for the test itself to add --enable-preview, so maybe that conflict will go away. But I still don't think this is the approach we should take (see my previous paragraph). ------------- PR: https://git.openjdk.org/jdk19/pull/88
