code review for JLI/JPLIS test (7191322)

[serguei.spitsyn at oracle.com]

New CR is:
   7191786 retransformClasses() does not pass in LocalVariableTypeTable 
of a method

Thanks,
Serguei

On 8/15/12 2:44 PM, serguei.spitsyn at oracle.com wrote:
> Dan,
>
> I'll file a new CR to cover the LVTT issue.
> As I remember correctly, the LVTT issue was out of the 7064927 scope.
>
> Thanks,
> Serguei
>
>
> On 8/15/12 1:47 PM, Daniel D. Daugherty wrote:
>> Serguei,
>>
>> Thanks for the quick review!
>>
>> Replies embedded below.
>>
>>
>> On 8/15/12 1:16 PM, serguei.spitsyn at oracle.com wrote:
>>> Dan,
>>>
>>> Thank you for doing this!
>>>
>>> Does the fix for 7064927 passes in LVTT tables as well?
>>
>> I don't think so, but that would be a question for Thomas
>> and/or Coleen.
>>
>>
>>> If so, then LVTT need to be tested too (is it right that just LVT is 
>>> tested?).
>>
>> Yes, my test only verifies LVT and does not verify LVTT.
>>
>>
>>> If not, then do we have a corresponding CR filed?
>>
>> I'm not aware of an open bug for LVTT, but I haven't looked for one.
>> I'm just closing out old, dangling threads from my Serviceability
>> days...
>>
>>
>>> The test looks good in general.
>>
>> Thanks!
>>
>>
>>> Just one minor question:
>>>
>>> Q1: The line VerifyLocalVariableTableOnRetransformTest.java: 103 
>>> contains a long
>>>    web link referencing the jvaweb.sfbay but the link is not 
>>> resolved in the browser.
>>>    Do we have to correct the link or, maybe, get rid of it?
>>
>> Bad Dan! The correct URL should be:
>>
>> http://docs.oracle.com/javase/7/docs/api/java/lang/instrument/Instrumentation.html#retransformClasses%28java.lang.Class...%29 
>>
>>
>> I will fix that.
>>
>> Again, thanks for the quick review!
>>
>> Dan
>>
>>
>>
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 8/15/12 9:58 AM, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> I wrote a test for the following bug:
>>>>
>>>>     7064927 4/4 retransformClasses() does not pass in
>>>>                 LocalVariableTable of a method
>>>>
>>>> a long time ago. 7064927 was fixed in the hotspot repo back in
>>>> HSX-23-B09 by Thomas W. and Coleen, but the test was never pushed
>>>> to the JDK repo. The java.lang.instrument (JLI) tests live in the
>>>> JDK repo.
>>>>
>>>> I'm using the following bug:
>>>>
>>>>     7191322 4/4 add test for 7064927 to java.lang.instrument
>>>>
>>>> to get the test into the JDK8 T&L repo. Here is the webrev URL:
>>>>
>>>>     http://cr.openjdk.java.net/~dcubed/7191322-webrev/0/
>>>>
>>>> Thanks, in advance, for any comments.
>>>>
>>>> Dan
>>>>
>>>> P.S.
>>>> The new test has been executed and passes on all platforms
>>>> supported by JPRT except for MacOS X. There is a temporary
>>>> build issue on MacOS X at the moment.
>>>
>>>
>
>