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

[JC Beyler]

Hi Alex,

I also saw two nits, in the same file as Chris was mentioning (
http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.01/test/jdk/com/sun/jdi/RedefineTTYLineNumber.java.udiff.html),
there is a comment:

// 11 before, 10 afterward

That is right below, we could remove it now, no?


Also in http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.01/test/jdk/com/sun/jdi/StringConvertTest.java.udiff.html:

+        // An unreported bug: this isn't handled correctly.
+        // If we uncomment this line, we will get an 'instance of...'  line
+        // which will cause the test to fail.
+        // jdb.command(JdbCommand.print("(Object)(StringConvertTarg.x3)"));


Should we perhaps file that bug now?


Apart from that, looks good to me!


Thanks!

Jc


On Fri, Sep 14, 2018 at 2:28 PM Chris Plummer <chris.plummer at oracle.com>
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
>
>
>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180914/c7b6bd65/attachment-0001.html>