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