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