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: serguei.spit...@oracle.com [mailto:serguei.spit...@oracle.com] Sent: Tuesday, June 21, 2016 6:49 AM To: Alan Bateman <alan.bate...@oracle.com>; serviceability-dev@openjdk.java.net; Christian Törnqvist <christian.tornqv...@oracle.com>; Alexander Kulyakhtin <alexander.kulyakh...@oracle.com>; hotspot-...@openjdk.java.net Developers <hotspot-...@openjdk.java.net> 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, serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com> 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
