jmx-dev RFR: 8010285 Enforce the requirement of Management Interfaces being public

[Daniel Fuchs]

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.
>>>
>>>