[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]

Igor,

I changed the webrev: http://cr.openjdk.java.net/~kshefov/8066798/webrev.02

-Konstantin

On 09.12.2014 15:03, Igor Ignatyev wrote:
> 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
>>>
>>