RFR: JDK-8210560: [TEST] convert com/sun/jdi redefineClass-related tests

[JC Beyler]

Hi Alex,

Looks great to me, thanks for addressing those two points!
Jc

On Tue, Sep 11, 2018 at 12:28 PM Alex Menkov <alexey.menkov at oracle.com>
wrote:

> Hi Jc,
>
> Thank you for review.
> Updated webrev (fixed both issue noted):
> http://cr.openjdk.java.net/~amenkov/sh2java/redefineClasses1/webrev.02/
>
> --alex
>
> On 09/10/2018 21:00, JC Beyler wrote:
> > Hi Alex,
> >
> > I was looking at this and I have a few nits and questions:
> > - I saw two spots in the patch that had weird spaces:
> > test/jdk/com/sun/jdi/RedefineG.java
> >
> > +        super(  DEBUGGEE_CLASS,
> > +                "RedefineG.java");
> >
> > - Then test/jdk/com/sun/jdi/RedefineImplementor.java
> >
> > +        super(  RedefineImplementorTarg.class.getName(),
> > +                "RedefineImplementor.java");
> >
> >
> > - My only question is should we not be worried or care if both
> test/jdk/com/sun/jdi/lib/jdb/JdbTest.java and
> test/jdk/com/sun/jdi/lib/jdb/ClassTransformer.java have now ways that could
> provoke NPEs? Should the code not try to throw an exception and say:
> careful, you called this but didn't call that first? It would make misuse
> of the class easier to debug when writing new tests.
> >
> > (Example being: calling redefineClass but not using the right
> constructor)
> >
> > I know it is for testing only but if we start writing new tests, it
> could be useful to be a bit defensive?
> >
> >
> > Apart from that, looked good to me.
> >
> > Jc
> >
> >
> >
> > On Mon, Sep 10, 2018 at 4:40 PM Alex Menkov <alexey.menkov at oracle.com
> > <mailto:alexey.menkov at oracle.com>> wrote:
> >
> >     Hi,
> >
> >     Please review a fix for
> >     https://bugs.openjdk.java.net/browse/JDK-8210560
> >     webrev:
> >
> http://cr.openjdk.java.net/~amenkov/sh2java/redefineClasses1/webrev.01/
> >     <
> http://cr.openjdk.java.net/%7Eamenkov/sh2java/redefineClasses1/webrev.01/>
> >
> >     New class (ClassTransformer) was developed to implement simple class
> >     transformer for class redefinition (the same functionality as
> >     implemented by ShellScaffold.sh).
> >
> >     Both setBreakpoint and redefineClass require source file name, so
> >     JdbTest class was updated to accept it as ctor argument and use by
> >     corresponding methods.
> >
> >     --alex
> >
> >
> >
> > --
> >
> > Thanks,
> > Jc
>


-- 

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