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

on last thing to think about:
does it make sense to move 'if (!run.passed) { ... } else { ... }' code 
into TestRun class?


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

-- 
Igor