code review for JLI/JPLIS test (7191322)

[serguei.spitsyn at oracle.com]

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.
>>
>>