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

[Alex Menkov]

On 09/14/2018 15:03, JC Beyler wrote:
> 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 
> <http://cr.openjdk.java.net/%7Eamenkov/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 inhttp://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.01/test/jdk/com/sun/jdi/StringConvertTest.java.udiff.html 
> <http://cr.openjdk.java.net/%7Eamenkov/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?

The log from 2 print calls:

 > print ((Object)StringConvertTarg.x3).toString()
[jdb] com.sun.tools.example.debug.expr.ParseException: No instance field 
or method with the name toString in StringConvertTarg$JJ2[]
[jdb]  ((Object)StringConvertTarg.x3).toString() = null
[jdb] main[1]
 > print (Object)(StringConvertTarg.x3)
[jdb]  (Object)(StringConvertTarg.x3) = instance of 
StringConvertTarg$JJ2[2] (id=624)
[jdb] main[1]

To me it looks correct as StringConvertTarg.x3 is an array.
The test is for 4511950 and 4843082, there is a comment for the 2nd one:
---
This fix also reverts the behavior of the command
print array
to the 1.4.1 behavior which is incorrect, but is better than the behavior
caused by the fix for 4511950.
---
So I suppose fix for 4511950 introduced some other output for arrays 
which was reverted later.
So I think the comment may be removed.

--alex

> 
> 
> 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 
> <mailto: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/
>     <http://cr.openjdk.java.net/%7Eamenkov/sh2java/step4/webrev.01/>
>      >
>      > --alex
> 
> 
> 
> 
> 
> -- 
> 
> Thanks,
> Jc