On 6/18/13 1:27 PM, Jaroslav Bachorik wrote:
On Tue 18 Jun 2013 12:30:07 PM CEST, Jaroslav Bachorik wrote:
On Tue 18 Jun 2013 12:25:35 PM CEST, Daniel Fuchs wrote:
Hi Jaroslav,
I've added the tests for the proper behaviour when the
"com.sun.jmx.mbeans.allowNonPublic" system property is set to true.
http://cr.openjdk.java.net/~jbachorik/8010285/webrev.05/
Thanks for adding the test. I haven't looked at the source again,
I trust nothing changed there. Some issues in the tests:
MBeanFallbackTest.java:
57 if (failures > 0) {
58 System.exit(1);
59 }
I think
throw new Exception("TEST FAILURES: " + failures);
would be more appropriate.
I saw both of them used across the regtests and was not really sure
which is the recommended way of signalling the test failure ...
Also I see that you have an @run main line - I believe it
should be @run main/othervm since the test sets some
system properties.
Currently all the testst in javax/management are run as main/othervm.
But it won't hurt to make the intention of running the test in a
separate JVM visually clear.
The same applies to the rest of the new tests.
MXBeanFallbackTest.java:
30 * @author Eamonn McManus
Yep :/
http://cr.openjdk.java.net/~jbachorik/8010285/webrev.06
Unified the way the test failure is reported, removed the forgotten
debug code and copy/paste errors.
I haven't changed "@run main" to "@run main/othervm" since the tests in
javax/management are run in othervm anyway. Also, there is more JMX
tests which eg. start they own MBean server and they don't declare "@run
main/othervm". So I'd prefer staying consistent.
-JB-
Looks good! But I'm not a reviewer...
-- daniel
-JB-
Is that a copy paste issue?
47 System.in.read();
Left over from debugging session - right ;-) ?
I see that you have an @run main line there too - I
believe it should be @run main/othervm since this
test also sets some system properties.
