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

[David Holmes]

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

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