Review request for 8022208: Intermittent test failures in java/lang/Thread/ThreadStateTest.java

[Mandy Chung]

On 11/5/2013 5:07 PM, David Holmes wrote:
> Thanks Mandy.
>
> One minor nit:
>
> Having goSleep be an instance method on ThreadStateController makes 
> the usage look weird:
>
>   thread.goSleep(10); // actually makes current thread go sleep
>

Good catch.  I changed it to a static pause(long ms) method. Changeset 
pushed.

thanks for the review.
Mandy


> I suggest making it static and use static import to shorten the call 
> site :)
>
> If you do change that, no need for re-review.
>
> David
>
> On 6/11/2013 7:58 AM, Mandy Chung wrote:
>> On 11/4/2013 8:42 PM, David Holmes wrote:
>>> This looks good to me. One nit that caused me some confusion - it took
>>> me a while to realize that in transitionTo eg:
>>>
>>>  221     public void transitionTo(Thread.State tstate) throws
>>> InterruptedException {
>>>  222         switch (tstate) {
>>>  223             case RUNNABLE:
>>>  224                 nextState(RUNNABLE);
>>>  225                 break;
>>>
>>> The case value, eg RUNNABLE, and the arg to nextState, eg RUNNABLE,
>>> are two completely different values! Can I suggest using S_xxx for the
>>> int states (why not an enum?).
>>>
>>
>> Good suggestion to rename them.  I considered adding an enum class that
>> might confuse with Thread.State enum and so decided to leave them as 
>> it is.
>>
>>> Typo: awaitArrive should be awaitAdvance
>>
>> Fixed.
>>
>> This updated webrev also fixes ThreadMXBeanStateTest to retry getting
>> ThreadInfo and improves the formatting of the output and include thread
>> ID in the output:
>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8022208/webrev.03/
>>
>> This version has got 1000 successful runs of both tests with and without
>> -Xcomp.
>>
>> thanks for the review.
>> Mandy