RFR: 8353955: nsk/jdi tests should be fixed to not always require includevirtualthreads=y

[Chris Plummer]

On Fri, 11 Apr 2025 23:23:52 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

> This is just a preliminary review. I'd like to get some approval for the approach I'm taking. There are over 300 tests that need to be fixed. I've just fixed a handful in this PR to give a feel for the changes I plan on making. If they look ok to you, then I'll update this PR with the needed changes to the rest of the tests.
> 
> What this PR is fixing is the issue with all of our nsk/jdi testing being done with includevirtualthreads=y even though debuggers typically use the default includevirtualthreads=n. As a result we have a testing gap with includevirtualthreads=n. There are nearly 1200 nsk/jdi tests. Only about 350 actually need includevirtualthreads=y. I plan making includevirtualthreads=n the default for nsk/jdi tests unless the test does something to override the default and request includevirtualthreads=y.
> 
> includevirtualthreads=y forces the debug agent to track all virtual threads so they are returned by vm.allThreads(). Some tests need this since they use vm.allThreads() to find the debuggee threads. Without includevirtualthreads=y, vm.allThreads() usually won't return any virtual threads (although it might return some for which events have been triggered).

Regarding the SerialExecutionDebugger change, some background might help. The following tests all use SerialExecutionDebugger:

 vmTestbase/nsk/jdi/stress/serial/forceEarlyReturn001/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/forceEarlyReturn002/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/heapwalking001/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/heapwalking002/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/mixed001/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/mixed002/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/monitorEvents001/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/monitorEvents002/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/ownedMonitorsAndFrames001/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/ownedMonitorsAndFrames002/TestDescription.java

Each of these tests has a set of existing tests (subtests) that will be executed using the same debugger and debuggee processes. They are executed either in a random order (shuffle) or for a large number of iterations, both with a goal of being somewhat stressful. There is a .tests file that specifies the subtests to run and how to run them. For example, for vmTestbase/nsk/jdi/stress/serial/forceEarlyReturn002 we have:

OPTIONS:shuffle
nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn001.forceEarlyReturn001
nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn002.forceEarlyReturn002
nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn003.forceEarlyReturn003
nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn004.forceEarlyReturn004
nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn005.forceEarlyReturn005
nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn008.forceEarlyReturn008
nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn013.forceEarlyReturn013
nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn014.forceEarlyReturn014

Some of the subtests requiring launching the debug agent with includevirtualthreads=y. Usually the subtest would be managing this and making sure the debug agent is launched with includevirtualthreads=y. However, for subtests run by the SerialExecutionDebugger, it is the SerialExecutionDebugger class that is responsible launching the debug agent, not the test, and therefore the subtest has no way to enforce its requirement for includevirtualthreads=y. It turns out there is only subtest that needs includevirtualthreads=y:

nsk.jdi.ThreadReference.forceEarlyReturn.forceEarlyReturn001.forceEarlyReturn002

You can see the fix for this test in this PR, but as pointed out above, this will only work when these test is run individually, not when run by SerialExecutionDebugger. Since SerialExecutionDebugger launches the debuggee, it needs to know whether or not to specify includevirtualthreads=y. At first I had it just always do that, but it was unnecessary for most of the tests, and reduced test coverage with includevirtualthreads=n. I then opted to add support for passing -includevirtualthreads to SerialExecutionDebugger. That way it's only necessary for the tests that include forceEarlyReturn002. If I want to take this further, I could add an additional @run command that does not use -includevirtualthreads, and also has a separate .test list that excludes forceEarlyReturn002, but I don't think this adds enough benefit to be worth while.

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

PR Comment: https://git.openjdk.org/jdk/pull/24606#issuecomment-2798204267