On 11/09/2013 5:33 PM, shanliang wrote:
David Holmes wrote:
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?
The version 01 is correct.
The version 00 was a mistake, yesterday I had some problems to generate
the version 01, so the version 00 was re-generated by mistake with
another fix, sorry for this confusing.
Ok functional change seems fine.
Looking at the tests a few minor nits:
test/javax/management/openmbean/OpenMBeanInfoEqualsNPETest.java
163 obj1.equals(null);
...
184 obj1.equals(null);
I think line 163 can be deleted.
In both tests ...
Use spaces around the + operator in strings eg:
"OK-1: "+obj1.get...
The message "with a null object" isn't quite reflective of what is being
tested. "null attribute" or even "null field" perhaps?
When you catch NPE and report an error it would be good to also print
the NPE stacktrace so we can see where we hit the unexpected NPE.
The tests are a little verbose ie printing each successful test result,
but that might be normal style in this area.
David
-----
Thanks,
Shanliang
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
