jmx-dev Codereview request: 8025206 IIntermittent test failure: javax/management/monitor/NullAttributeValueTest.java

Jaroslav Bachorik jaroslav.bachorik at oracle.com
Mon Sep 30 00:56:02 PDT 2013


Hi,

On 30.9.2013 09:46, shanliang wrote:
> Thanks Jaroslav for the review, here is the new version with the
> debugging info:
>     http://cr.openjdk.java.net/~sjiang/JDK-8025206/01/

I'm fine with the changes.

-JB-

>
> Shanliang
>
> Jaroslav Bachorik wrote:
>> Hi Shanliang,
>>
>> On 30.9.2013 08:55, shanliang wrote:
>>> Hi,
>>>
>>> Please review this test fix, I set a much long waiting time to receive a
>>> notification, but not simply wait the test timeout, in order to have
>>> less modification.
>>
>> IMO, the change to let the harness terminate the test on timeout was
>> quite readable as well. But you leave more than sufficient buffer for
>> any delays in delivering notifications so I suppose it should be fine.
>>
>> You are missing the debugging echos in places where you extracted the
>> checkReceived() method.
>>
>> And just a small nit - on line 31 you should probably add the @author
>> tag.
>>
>> Cheers,
>>
>> -JB-
>>
>>>
>>> webrev: http://cr.openjdk.java.net/~sjiang/JDK-8025206/00/
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8025206
>>>
>>> Thanks,
>>> Shanliang
>>
>



More information about the jmx-dev mailing list