jmx-dev Codereview request: 8023529 OpenMBeanInfoSupport.equals/hashCode throw NPE

[David Holmes]

I'm a bit confused. The two webrevs are vastly different:

http://cr.openjdk.java.net/~sjiang/jdk-8023529/01/

http://cr.openjdk.java.net/~sjiang/jdk-8023529/00/

Should they be combined somehow?

David
-----

On 11/09/2013 12:15 AM, shanliang wrote:
> Jaroslav Bachorik wrote:
>> On 09/05/2013 10:37 AM, shanliang wrote:
>>
>>> I have added 2 tasts to make sure that call
>>> OpenMBean*InfoSupport.equals/hashCode do not throw NPE
>>> The unit tests and JCK tests are passed.
>>>
>>> Webrev:http://cr.openjdk.java.net/~sjiang/JDK-8023529/00/
>>> Bug:https://bugs.openjdk.java.net/browse/JDK-8023529
>>>
>>
>> javax/management/openmbean/OpenMBeanInfoEqualsNPETest.java
>>
>> * rename the "obj1", "obj2" parameters to something more explicit - eg.
>> "referenceInfo" and "nullValueinfo" or something similar
>>
> No reference object here, the test has to call both obj1.equals and
> obj2.equals.
>> * when testing "obj.equals(null);" the messages should not contain
>> reference to the parameter since you are not, in fact, testing the null
>> value of the given parameter but rather a null info object
>> * the message "got NPE if null paramer" should read "got NPE for null
>> paramter"
>>
> Right, should be "a null object" instead of "a null parameter."
>> * "Thread.sleep(100);" on line 153 is necessary?
>>
> It is not necessary for the test, removed now.
>> -----
>>
>> javax/management/openmbean/OpenMBeanInfoHashCodeNPETest.java
>>
>> * the message "got NPE if null paramer" should read "got NPE for null
>> paramter"
>> * "Thread.sleep(100);" on line 143 is necessary?
>>
> Thanks for the review, here is the new version:
>
> http://cr.openjdk.java.net/~sjiang/jdk-8023529/01/
>
> Shanliang
>>
>> -JB-
>>
>>
>>> Thanks,
>>>
>>> Shanliang
>>>
>>
>>
>