[9] Review request : JDK-8066798: TEST_RFR: Make java/lang/invoke/LFCaching tests use lib/testlibrary/jdk/testlibrary/TimeLimitedRunner.java to define their number of iterations

[Konstantin Shefov]

Hi Igor

Thanks for reviewing.
I have made some changes, added a separate class to store the variables. 
"doneIterations" is not local for lambda, it is incremented after each 
lambda execution.
Also I changed to catch Exception.

http://cr.openjdk.java.net/~kshefov/8066798/webrev.01

-Konstantin

On 09.12.2014 12:28, Igor Ignatyev wrote:
> Hi Konstantin,
>
> 0. I don't like static variables which you introduced. were I you, I'd 
> create a class which encapsulates them.
> + it looks like
>  - doneIterations is a local for the lambda
>  - totalIterations is almost affectively final
>  - testCounter/failCounter are just informative variable and can be 
> omitted
> so you just need a way to store/get passed value. that can be done via 
> mutable box, e.g. new boolean[1], AtomicBoolean.
>
> 1. why do you catch Throwable? please catch the most specific exception
> 2. at lines 174-176, you lose exception information, it's better not 
> to print exception's stack trace, but pass the exception to ctor of 
> Error.
>
> Thanks,
> Igor
>
> On 12/09/2014 11:51 AM, Konstantin Shefov wrote:
>> Hello,
>>
>> Please review the test enhancement
>> https://bugs.openjdk.java.net/browse/JDK-8066798
>> Webrev is http://cr.openjdk.java.net/~kshefov/8066798/webrev.00
>>
>> Test has been modified to use
>> lib/testlibrary/jdk/testlibrary/TimeLimitedRunner.java class to define
>> its number of iterations depending on the given timeout.
>> Also @ignore tag has been removed from
>> test/java/lang/invoke/LFCaching/LFGarbageCollectedTest.java because
>> JDK-8057020 has been fixed in JDK 9.
>>
>> Thanks
>>
>> -Konstantin
>