Hi Serguei,
I’ve only looked at the test code so far, here are some comments: * You’re not allowed to call System.exit in a jtreg test. If something fails, you need to throw an exception. * Instead of a run() method you could simply collapse all this into the main method like this: assertEQ(check(), 0); * Would it make sense to throw exceptions with useful error messages from the JNI code instead of returning failed status codes? Our error matching system would work a lot better with unique error messages. Thanks, Christian From: [email protected] [mailto:[email protected]] Sent: Tuesday, June 21, 2016 6:49 AM To: Alan Bateman <[email protected]>; [email protected]; Christian Törnqvist <[email protected]>; Alexander Kulyakhtin <[email protected]>; [email protected] Developers <[email protected]> Subject: Re: Jigsaw Enhancement RFR: 8159145 Add JVMTI function GetModuleByPackageName On 6/21/16 03:23, Alan Bateman wrote: On 21/06/2016 10:54, [email protected] <mailto:[email protected]> wrote: Please, review the Jigsaw fix for the enhancement: https://bugs.openjdk.java.net/browse/JDK-8159145 The Hotspot webrev: http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/ The Jdk webrev: http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/ Serguei - can you put the patches on cr.openjdk.java.net? Sorry. My initial plan was to write an nsk.jvmti test and review it on a confidential mailing list. It is why I put the webrevs on the non-public server. Forgot to switch the server when the test was converted into the jtreg format. The public webrevs are: Hotspot: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/ Jdk: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/ Thanks, Serguei -Alan
