[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

[Igor Ignatyev]

Konstantin,

well, now I don't see any points to have a lambda. you can simple make 
it a method in TestRunInfo and rename TestRunInfo into TestRun.

>  174                             t.printStackTrace();
>  175                             System.err.println("FAILED");
you don't print exception message.

-- 
Igor

On 12/09/2014 02:19 PM, Konstantin Shefov wrote:
> 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
>>
>