RFR: JDK-8210560: [TEST] convert com/sun/jdi redefineClass-related tests
Alex Menkov
alexey.menkov at oracle.com
Tue Sep 11 19:28:15 UTC 2018
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
More information about the serviceability-dev
mailing list