RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4
Chris Plummer
chris.plummer at oracle.com
Thu Sep 20 22:07:23 UTC 2018
Hi Alex,
Can you put "expected statement printed by jdb" in a static final since
it is used in 2 places? Otherwise it looks good. I don't need to see an
updated webrev.
thanks,
Chris
On 9/20/18 10:24 AM, Alex Menkov wrote:
> 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
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
More information about the serviceability-dev
mailing list