Re: RFR(M): 8178054: [TESTBUG] Need test for JVM TI IsModifiableModule

[serguei.spit...@oracle.com]

I've filed a test enhancement and targeted it to tbd_minor:   https://bugs.openjdk.java.net/browse/JDK-8182130     update the IsModifiableModule test if any module becomes non-modifiable Thanks, Serguei On 6/13/17 15:20, Igor Ignatyev wrote: I see. I asumme that such test will be added when we introduce such modules and update IsModifiableModule method, however I'd recommend to file an RFE so we won't forget about it later. Cheers, -- Igor On Jun 13, 2017, at 3:12 PM, serguei.spit...@oracle.com wrote: Hi Igor, Thank you a lot for review! There are no non-modifiable modules in jdk 9. So that it is impossible to test that IsModifiableModule returns false. Thank you for the suggestion, I'll replace the Exception with java/lang/AssertionError. Thanks, Serguei On 6/13/17 15:07, Igor Ignatyev wrote: 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 wrote: On 6/10/17 00:41, Alan Bateman wrote: On 09/06/2017 19:32, 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/ I think it looks good now. Thank you a lot for review and good suggestions! Serguei -Alan.