RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4
Alex Menkov
alexey.menkov at oracle.com
Thu Sep 20 23:11:14 UTC 2018
Hi Chris,
On 09/20/2018 15:07, Chris Plummer wrote:
> 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.
Not sure how it can be done.
The text has to be explicitly specified in the 1st line of
RedefineTTYLineNumberTarg.A method (as jdb prints the source).
--alex
>
> 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