You didn't print the NPE stacktrace in OpenMBeanInfoHashCodeNPETest.java
Otherwise ok.
No need to rereview if you add the stacktrace printing.
Thanks,
David
On 11/09/2013 10:32 PM, shanliang wrote:
David Holmes wrote:
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.
Yes.
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.
OK.
The tests are a little verbose ie printing each successful test
result, but that might be normal style in this area.
Here we need to test all fields so I print out what we test, so easy to
check no missed.
I get used to add more info, because sometime we miss info when a test
fails.
Here is the new version:
http://cr.openjdk.java.net/~sjiang/jdk-8023529/02/
Thanks,
Shanliang
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
