Hi Serguei,
>Will check if this works well in this case. >The unique error messages are already printed in the native code. Yes, they’re printed on stdout/stderr but the test will fail with a “Test failed” exception and our failure matching system wouldn’t be able to tell why it failed. It’d be a lot nicer if it’d just throw an exception when one of the checks in the native code failed. Thanks, Christian From: serguei.spit...@oracle.com [mailto:serguei.spit...@oracle.com] Sent: Tuesday, June 21, 2016 10:59 AM To: Christian Tornqvist <christian.tornqv...@oracle.com>; 'Alan Bateman' <alan.bate...@oracle.com>; serviceability-dev@openjdk.java.net; 'Alexander Kulyakhtin' <alexander.kulyakh...@oracle.com>; hotspot-...@openjdk.java.net Subject: Re: Jigsaw Enhancement RFR: 8159145 Add JVMTI function GetModuleByPackageName Hi Christian, Thank you for looking at it! On 6/21/16 04:56, Christian Tornqvist wrote: 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. Ok, an exception is Ok with me. * Instead of a run() method you could simply collapse all this into the main method like this: assertEQ(check(), 0); Will check if this works for me. * 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. Will check if this works well in this case. The unique error messages are already printed in the native code. Thanks, Serguei Thanks, Christian From: serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com> [mailto:serguei.spit...@oracle.com] Sent: Tuesday, June 21, 2016 6:49 AM To: Alan Bateman <mailto:alan.bate...@oracle.com> <alan.bate...@oracle.com>; serviceability-dev@openjdk.java.net <mailto:serviceability-dev@openjdk.java.net> ; Christian Törnqvist <mailto:christian.tornqv...@oracle.com> <christian.tornqv...@oracle.com>; Alexander Kulyakhtin <mailto:alexander.kulyakh...@oracle.com> <alexander.kulyakh...@oracle.com>; hotspot-...@openjdk.java.net <mailto:hotspot-...@openjdk.java.net> Developers <mailto:hotspot-...@openjdk.java.net> <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/ <http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/> Jdk: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/ <http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/> Thanks, Serguei -Alan
