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

[Igor Ignatyev]

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.spitsyn at 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.spitsyn at oracle.com <mailto:serguei.spitsyn at 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.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> wrote:
>>>> 
>>>> 
>>>> On 6/10/17 00:41, Alan Bateman wrote: 
>>>>> 
>>>>> 
>>>>> On 09/06/2017 19:32, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at 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/%7Esspitsyn/webrevs/2017/hotspot/8178054-jvmti-ismodif.4/> 
>>>>> I think it looks good now. 
>>>> 
>>>> Thank you a lot for review and good suggestions! 
>>>> Serguei 
>>>> 
>>>>> 
>>>>> -Alan. 
>>>> 
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20170613/07bb5115/attachment.html>