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

JC Beyler jcbeyler at google.com
Tue Sep 11 04:00:21 UTC 2018


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>
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/
>
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180910/79f51733/attachment.html>


More information about the serviceability-dev mailing list