openjdk
8327704: Update nsk/jdi tests to use driver instead of othervm
vmtestbase nsk/jdi tests run 2 processes: debugger and debugee. There is not need to start debugger in the separate process for each test. Also, no need to run it with "-Xcomp" because the main goal is to test debugee behavior with different VM flags. This fix updates tests to run debugger as driver to optimize execution time. The fix also eliminates System.exit() which is not compatible with driver/agentvm mode and update shared classes to correctly add classpath when running debugee. There were few tests which still execute using othervm mode. They require some specific classpath/settings. Tested by running all tests, with '-Xcomp' and virtual thread test factory.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8327704: Update nsk/jdi tests to use driver instead of othervm (Sub-task - P4)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18174/head:pull/18174
$ git checkout pull/18174
Update a local copy of the PR:
$ git checkout pull/18174
$ git pull https://git.openjdk.org/jdk.git pull/18174/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 18174
View PR using the GUI difftool:
$ git pr show -t 18174
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18174.diff
Webrev
:wave: Welcome back lmesnik! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@lmesnik The following label will be automatically applied to this pull request:
-
serviceability
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.
If you missed removing a System.exit(), what would the symptoms be if called?
Can you give us examples of before and after failure modes?
If you missed removing a System.exit(), what would the symptoms be if called?
Can you give us examples of before and after failure modes?
The test which is executed as driver and call System.exit() fails with following message:
Debuggee's process finished with status: 95
TEST RESULT: Error. Agent communication error: java.io.EOFException; check console log for any additional details
There is not need to start debugger in the separate process for each test.
I don't think I agree with that. Don't we want to have the debugger in a well known initial state for each test?
That's a good question. Each test creates a new connector, starts new vm process with debugee and launch com.sun.jdi.VirtualMachine. I expect that it should be fine to run all of them in same vm. We are already use the same mode for com/sun/jdi jdk regression tests. Moreover, it should be a bug if we can't run all these tests in the same vm process.
But if one test has a problem and messes up the debugger won't that kill the rest of the tests?
@lmesnik This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8327704: Update nsk/jdi tests to use driver instead of othervm
Reviewed-by: sspitsyn, cjplummer
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been 211 new commits pushed to the master branch:
- cc5cda558736ffeae2a4611d87dfe280b9a4eac3: 8328005: Convert java/awt/im/JTextFieldTest.java applet test to main
- ef2bd5721316f208a458fecfffa02ec2c0ea7fe0: 8328558: Convert javax/swing/JCheckBox/8032667/bug8032667.java applet test to main
- e66788c16563d343f6cccd2807a251ccc6f9b64a: 8325179: Race in BasicDirectoryModel.validateFileCache
- 1496b5de9004c6a9e43ead5f6e9e88362028a7cc: 8327924: Simplify TrayIconScalingTest.java
- 16ed191329b517766313761c8f3450b4a8285658: 8328101: Parallel: Obsolete ParallelOldDeadWoodLimiterMean and ParallelOldDeadWoodLimiterStdDev
- 581b1e29aebd425bade14d2ee46704a16187df5b: 8328402: Implement pausing functionality for the PassFailJFrame
- 75195aab497a2d23548128e03f6887283dcaa7e1: 8328299: Convert DnDFileGroupDescriptor.html applet test to main
- 725d87bbc2abae2ca856d91668f0a494f93fca1c: 8328570: Convert closed JViewport manual applet tests to main
- e41bc42deb22615c9b93ee639d04e9ed2bd57f64: 8327990: [macosx-aarch64] Various tests fail with -XX:+AssertWXAtThreadSync
- 43080173e88c8f53cd54c9096c79f3144007fd97: 8328631: Convert java/awt/InputMethods/InputMethodsTest/InputMethodsTest.java applet test to manual
- ... and 201 more: https://git.openjdk.org/jdk/compare/87b40c6ad2b0fa972fa6c5699a52045e82e0c7ef...master
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.
That's a more generic question. The driver and main (default mode) actions might be executed in 2 modes: agentvm or othervm. So each driver (or main) action might reuse jvm or start a new one each time. This is controlled by -agetnvm flag in jtreg. While othervm mode in tests is used for tests that couldn't be executed in agentvm mode (use some specific VM options). We don't force /othrevm option in tests because something might be broken and affect other tests execution. (Really it means that every test that doesn't start new process should be othrevm). Each time when we run jtreg in agentvm we allow tests to reuse the same JVM and taking risks of possible incosistency of the future tests execution. So if test manages to break some part of JVM/JDK (it might be thread state, compile, gc, debugger,classoader, java.util or javac) we have a risk that next tests have jdk in inconsistent state.
I think that the debugger might be treated as any other component. If we need more isolation then it is needed to use othervm mode for jtreg.
And using of agentvm/othervm mode for VM testing might be separate topic.
I can do more testing in different modes to see if we have any new failures after update.
I agree there is a more general issue of test interference in anything other than othervm mode but that is beside the point. It seems to me that going from othervm to driver mode for the debugger has some not insignificant risk when it comes to interference.
There were few tests which still execute using othervm mode. They require some specific classpath/settings.
Which ones? They are hard to find among the 2000+ files changed.
There are othervm/native tests. I am not sure if they could be just converted. MonitorContendedEnteredRequest/addClassFilter_ClassName/TestDescription.java:64: * @run main/othervm/native MonitorContendedEnteredRequest/MonitorContendedEnteredRequest001/TestDescription.java:54: * @run main/othervm/native MonitorContendedEnteredRequest/MonitorContendedEnteredRequest002/TestDescription.java:54: * @run main/othervm/native MonitorContendedEnteredRequest/addClassFilter_ReferenceType/TestDescription.java:62: * @run main/othervm/native MonitorContendedEnteredRequest/addClassExclusionFilter/TestDescription.java:63: * @run main/othervm/native MonitorContendedEnteredRequest/addInstanceFilter/TestDescription.java:63: * @run main/othervm/native MonitorContendedEnteredRequest/addThreadFilter/TestDescription.java:63: * @run main/othervm/native MonitorContendedEnterRequest/addClassFilter_ClassName/TestDescription.java:64: * @run main/othervm/native MonitorContendedEnterRequest/addClassFilter_ReferenceType/TestDescription.java:62: * @run main/othervm/native MonitorContendedEnterRequest/MonitorContendedEnterRequest001/TestDescription.java:54: * @run main/othervm/native MonitorContendedEnterRequest/addClassExclusionFilter/TestDescription.java:63: * @run main/othervm/native MonitorContendedEnterRequest/addInstanceFilter/TestDescription.java:63: * @run main/othervm/native MonitorContendedEnterRequest/addThreadFilter/TestDescription.java:65: * @run main/othervm/native MonitorContendedEnterRequest/MonitorContendedEnterRequest002/TestDescription.java:54: * @run main/othervm/native MethodExitEvent/returnValue/returnValue004/returnValue004.java:50: * @run main/othervm/native ThreadReference/forceEarlyReturn/forceEarlyReturn005/forceEarlyReturn005.java:54: * @run main/othervm/native ThreadReference/forceEarlyReturn/forceEarlyReturn004/forceEarlyReturn004.java:42: * @run main/othervm/native ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames004/ownedMonitorsAndFrames004.java:48: * @run main/othervm/native ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames009/ownedMonitorsAndFrames009.java:47: * @run main/othervm/native ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames002/ownedMonitorsAndFrames002.java:52: * @run main/othervm/native ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames007/ownedMonitorsAndFrames007.java:54: * @run main/othervm/native ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames008/TestDescription.java:57: * @run main/othervm/native ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames006/ownedMonitorsAndFrames006.java:51: * @run main/othervm/native ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames003/ownedMonitorsAndFrames003.java:51: * @run main/othervm/native ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames005/ownedMonitorsAndFrames005.java:48: * @run main/othervm/native ReferenceType/instances/instances001/instances001.java:62: * @run main/othervm/native ReferenceType/instances/instances004/TestDescription.java:49: * @run main/othervm/native ReferenceType/instances/instances002/instances002.java:44: * @run main/othervm/native ReferenceType/instances/instances005/instances005.java:44: * @run main/othervm/native ReferenceType/instances/instances003/instances003.java:48: * @run main/othervm/native ObjectReference/referringObjects/referringObjects003/referringObjects003.java:60: * @run main/othervm/native ObjectReference/referringObjects/referringObjects002/referringObjects002.java:61: * @run main/othervm/native ObjectReference/referringObjects/referringObjects001/referringObjects001.java:60: * @run main/othervm/native ObjectReference/referringObjects/referringObjects004/referringObjects004.java:44: * @run main/othervm/native stress/MonitorEvents/MonitorEvents001/TestDescription.java:62: * @run main/othervm/native stress/MonitorEvents/MonitorEvents002/TestDescription.java:62: * @run main/othervm/native stress/serial/ownedMonitorsAndFrames002/TestDescription.java:61: * @run main/othervm/native stress/serial/mixed001/TestDescription.java:68: * @run main/othervm/native stress/serial/monitorEvents002/TestDescription.java:58: * @run main/othervm/native stress/serial/ownedMonitorsAndFrames001/TestDescription.java:57: * @run main/othervm/native stress/serial/mixed002/TestDescription.java:66: * @run main/othervm/native stress/serial/monitorEvents001/TestDescription.java:56: * @run main/othervm/native stress/serial/heapwalking002/TestDescription.java:60: * @run main/othervm/native stress/serial/heapwalking001/TestDescription.java:58: * @run main/othervm/native stress/serial/forceEarlyReturn002/TestDescription.java:66: * @run main/othervm/native stress/serial/forceEarlyReturn001/TestDescription.java:64: * @run main/othervm/native VirtualMachine/instanceCounts/instancecounts002/TestDescription.java:51: * @run main/othervm/native VirtualMachine/instanceCounts/instancecounts001/instancecounts001.java:50: * @run main/othervm/native
and a bunch of tests for launching/connecting. They customized connection. VirtualMachineManager/createVirtualMachine/createVM004/TestDescription.java:52: * @run main/othervm VirtualMachineManager/createVirtualMachine/createVM003/TestDescription.java:51: * @run main/othervm VirtualMachineManager/createVirtualMachine/createVM005/TestDescription.java:51: * @run main/othervm ListeningConnector/accept/accept002/TestDescription.java:48: * @run main/othervm ListeningConnector/accept/accept001/TestDescription.java:48: * @run main/othervm ListeningConnector/startListening/startlis001/TestDescription.java:55: * @run main/othervm ListeningConnector/startListening/startlis002/TestDescription.java:57: * @run main/othervm ListeningConnector/listennosuspend/listennosuspend001/TestDescription.java:42: * @run main/othervm AttachingConnector/attachnosuspend/attachnosuspend001/TestDescription.java:45: * @run main/othervm AttachingConnector/attach/attach002/TestDescription.java:52: * @run main/othervm AttachingConnector/attach/attach001/TestDescription.java:50: * @run main/othervm PlugConnectors/AttachConnector/plugAttachConnect002/plugAttachConnect002.java:68: * @run main/othervm PlugConnectors/AttachConnector/plugAttachConnect003/plugAttachConnect003.java:53: * @run main/othervm PlugConnectors/AttachConnector/plugAttachConnect001/plugAttachConnect001.java:60: * @run main/othervm PlugConnectors/ListenConnector/plugListenConnect002/plugListenConnect002.java:68: * @run main/othervm PlugConnectors/ListenConnector/plugListenConnect001/plugListenConnect001.java:60: * @run main/othervm PlugConnectors/ListenConnector/plugListenConnect003/plugListenConnect003.java:53: * @run main/othervm PlugConnectors/LaunchConnector/plugLaunchConnect003/plugLaunchConnect003.java:53: * @run main/othervm PlugConnectors/LaunchConnector/plugLaunchConnect002/plugLaunchConnect002.java:68: * @run main/othervm PlugConnectors/LaunchConnector/plugLaunchConnect001/plugLaunchConnect001.java:60: * @run main/othervm PlugConnectors/MultiConnectors/plugMultiConnect006/plugMultiConnect006.java:111: * @run main/othervm PlugConnectors/MultiConnectors/plugMultiConnect003/plugMultiConnect003.java:89: * @run main/othervm PlugConnectors/MultiConnectors/plugMultiConnect002/plugMultiConnect002.java:83: * @run main/othervm PlugConnectors/MultiConnectors/plugMultiConnect004/plugMultiConnect004.java:98: * @run main/othervm PlugConnectors/MultiConnectors/plugMultiConnect005/plugMultiConnect005.java:99: * @run main/othervm PlugConnectors/MultiConnectors/plugMultiConnect001/plugMultiConnect001.java:71: * @run main/othervm PlugConnectors/TransportService/transportService001/transportService001.java:77: * @run main/othervm PlugConnectors/TransportService/transportService003/transportService003.java:54: * @run main/othervm PlugConnectors/TransportService/transportService002/transportService002.java:77: * @run main/othervm Argument/value/value003/TestDescription.java:56: * @run main/othervm Argument/value/value004/TestDescription.java:55: * @run main/othervm LaunchingConnector/launchnosuspend/launchnosuspend001/TestDescription.java:42: * @run main/othervm LaunchingConnector/launch/launch002/TestDescription.java:50: * @run main/othervm LaunchingConnector/launch/launch001/TestDescription.java:50: * @run main/othervm LaunchingConnector/launch/launch004/TestDescription.java:50: * @run main/othervm LaunchingConnector/launch/launch003/TestDescription.java:50: * @run main/othervm
They also updated just to throw RuntimeException as other tests.
On Tue, Mar 19, 2024 at 2:20 PM Chris Plummer @.***> wrote:
So no changes were made to any of these tests?
— Reply to this email directly, view it on GitHub https://github.com/openjdk/jdk/pull/18174#issuecomment-2008154319, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOQDW4KAIFB5Q5BLKAROS3YZCTYFAVCNFSM6AAAAABENMXMB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBYGE2TIMZRHE . You are receiving this because you were mentioned.Message ID: @.***>
Going to push as commit 771f6a2f8e236e224b0e33b89f664c4bfc49e595.
Since your change was applied there have been 217 commits pushed to the master branch:
- 4210e507a0e65de00fab50e0584aa82cd7b3bf27: 8320362: Load anchor certificates from Keychain keystore
- ab183e437c18b445e9c022a4d74de818d4ccecbe: 8328642: Convert applet test MouseDraggedOutCauseScrollingTest.html to main
- ab28045d7785d948b2bce685f06043e8217961f4: 8328316: Finisher cannot emit if stream is sequential and integrator returned false
- ba05c6d0b6955a0c5110b75e709e2206961f9fec: 8328368: Convert java/awt/image/multiresolution/MultiDisplayTest/MultiDisplayTest.java applet test to main
- fa823bd21c6676c67c42275efb59868200409b38: 8328540: test javax/swing/JSplitPane/4885629/bug4885629.java fails on windows hidpi
- e81374e0501a8201dac701824d2e29f2cedfcdff: 8328560: java/awt/event/MouseEvent/ClickDuringKeypress/ClickDuringKeypress.java imports Applet
- cc5cda558736ffeae2a4611d87dfe280b9a4eac3: 8328005: Convert java/awt/im/JTextFieldTest.java applet test to main
- ef2bd5721316f208a458fecfffa02ec2c0ea7fe0: 8328558: Convert javax/swing/JCheckBox/8032667/bug8032667.java applet test to main
- e66788c16563d343f6cccd2807a251ccc6f9b64a: 8325179: Race in BasicDirectoryModel.validateFileCache
- 1496b5de9004c6a9e43ead5f6e9e88362028a7cc: 8327924: Simplify TrayIconScalingTest.java
- ... and 207 more: https://git.openjdk.org/jdk/compare/87b40c6ad2b0fa972fa6c5699a52045e82e0c7ef...master
Your commit was automatically rebased without conflicts.
@lmesnik Pushed as commit 771f6a2f8e236e224b0e33b89f664c4bfc49e595.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.