code review for JLI/JPLIS test (7191322)

[Daniel D. Daugherty]

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