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

[Alex Menkov]

Looks like only line numbers are reported correctly, but the content of 
the line content if not correct :)

[jdb] Breakpoint hit: "thread=main", RedefineTTYLineNumberTarg.A(), 
line=47 bci=0
[jdb] 47            System.out.println("in A, about to call B");
[jdb]
[jdb] main[1]

[jdb] Breakpoint hit: "thread=main", RedefineTTYLineNumberTarg.A(), 
line=46 bci=0
[jdb] 46        public void A() {
[jdb]
[jdb] main[1]

"public void A()" is a line 46 in the original file

--alex

On 09/14/2018 15:32, Chris Plummer wrote:
> I think checking the output after the second breakpoint would be a good 
> idea. However, rather than checking the line number, maybe just check 
> the contents of the line, which should be included in the breakpoint 
> event output.
> 
> Chris
> 
> On 9/14/18 3:23 PM, Alex Menkov wrote:
>> 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
>>>
>>>
>>>
> 
>