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

[Alex Menkov]

Looks like JDK-4660756 fixed line number, but didn't fix source.
I filled JDK-8210927 for this.

New webrev:
http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.02/
changes (vs ver .01):
- updated comment in StringConvertTest.java about arrays;
- RedefineTTYLineNumber was updated to verify line numbers,
   verification of the source at breakpoint is added, but commented out 
(should be uncommented by fix for JDK-8210927)

--alex

On 09/14/2018 18:09, Chris Plummer wrote:
> Hmm. I thought that's what the original bug was addressing.
> 
>    27  * @summary TTY: Need to clear source cache after doing a redefine 
> class
> 
> Chris
> 
> On 9/14/18 4:37 PM, Alex Menkov wrote:
>> 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
>>>>>
>>>>>
>>>>>
>>>
>>>
> 
>