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: [email protected] [mailto:[email protected]] Sent: Tuesday, June 21, 2016 10:59 AM To: Christian Tornqvist <[email protected]>; 'Alan Bateman' <[email protected]>; [email protected]; 'Alexander Kulyakhtin' <[email protected]>; [email protected] 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: [email protected] <mailto:[email protected]> [mailto:[email protected]] Sent: Tuesday, June 21, 2016 6:49 AM To: Alan Bateman <mailto:[email protected]> <[email protected]>; [email protected] <mailto:[email protected]> ; Christian Törnqvist <mailto:[email protected]> <[email protected]>; Alexander Kulyakhtin <mailto:[email protected]> <[email protected]>; [email protected] <mailto:[email protected]> Developers <mailto:[email protected]> <[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/ <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
