RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

[Alex Menkov]

Hi Chris,

The file history does not contain any info about line number dependency.
I'll remove "11 before, 10 afterward" comment.
Actually the test is not clear to me.
Accordingly the test description jdb report obsolete line number in the 
case, but the test does not verify its correctness, but just checks 
_debuggee_ (not jdb) output for absence of "Internal exception".
The original bug is ancient, so it's hard to say if the test is correct 
or not.
I can add extra testing - extract reported line numbers (by using regexp 
"line=(\d+)\b") and verify that 2st breakpoint is reported with the 
expected line number (1 less than line of the 1st breakpoint).
Thoughts?

--alex

On 09/14/2018 14:27, Chris Plummer wrote:
> Hi Alex,
> 
> Just one issue I see. For RedefineTTYLineNumber.java, the original test 
> used to have this comment, which your removed:
> 
>    52   // line number sensitive!!! Next line must be line 10.
> 
> It's not clear to me why this test was ever line number sensitive, and 
> whether you removed this sensitivity, or it just never existed. In any 
> case, you left in the following comment, which maybe should also be 
> removed:
> 
>    47         System.out.println("in A, about to call B");  // 11 
> before, 10 afterward
> 
> Also, the println output from A() does not seem to match what the test 
> is doing. There is no call to B():
> 
>    46     public void A() {
>    47         System.out.println("in A, about to call B");  // 11 
> before, 10 afterward
>    48         System.out.println("out from B");
>    49     }
> 
> Maybe that's some bit rot. My understanding of the point of the test is 
> while at the breakpoint at the start of A(), a redefine is done that 
> deletes a line above this point, and jdi needs to make the appropriate 
> adjustment of the current breakpoint line number. So calling B() does 
> not play a roll in this, but perhaps it did a one point but the call was 
> removed.
> 
> Also, I don't see any indication of line number sensitivity here, but 
> once again, maybe this is a bit rot issue and at one point it was line 
> number sensitive.
> 
> thanks,
> 
> Chris
> 
> On 9/14/18 12:59 PM, Alex Menkov wrote:
>> Hi all,
>>
>> please review fix for
>> https://bugs.openjdk.java.net/browse/JDK-8210760
>> webrev:
>> http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.01/
>>
>> --alex
> 
> 
>