HI Serguei, generally it looks good, but you still need at least one test to check that IsModifiableModule returns false than it must.
minor nit: I'd prefer to have java/lang/AssertionError instead of Exception as EXC_CNAME in libIsModifiableModuleTest.c as it clearly shows that some of test checks failed. Thanks, -- Igor > On Jun 13, 2017, at 1:56 PM, serguei.spit...@oracle.com wrote: > > PING.. > > This is for jdk 9. > I have one review from Alan and need at least one more. > Added hotspot-test and hotspot_dev_confidential mailing lists to the "To:" > list. > > Thanks, > Serguei > > > On 6/10/17 04:08, serguei.spit...@oracle.com > <mailto:serguei.spit...@oracle.com> wrote: >> >> >> On 6/10/17 00:41, Alan Bateman wrote: >>> >>> >>> On 09/06/2017 19:32, serguei.spit...@oracle.com >>> <mailto:serguei.spit...@oracle.com> wrote: >>>> >>>> Ok, I removed the get_class_loader function, replaced the loader with a >>>> class and left alone the "not_a_module" parameter. >>>> This is the simplest way to provide a non-module jobject. >>>> Otherwise, the JNIEnv parameter has to be passed to the >>>> check_is_modifiable_error_codes . >>>> Also, I've done a small refactoring by inlining the >>>> test_is_modifiable_module body into the *_check function. >>>> >>>> New webrev: >>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8178054-jvmti-ismodif.4/ >>>> >>>> <http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8178054-jvmti-ismodif.4/> >>>> >>> I think it looks good now. >> >> Thank you a lot for review and good suggestions! >> Serguei >> >>> >>> -Alan. >> >
