[9] Review request : JDK-8066798: [TEST] Make java/lang/invoke/LFCaching tests use lib/testlibrary/jdk/testlibrary/TimeLimitedRunner.java to define their number of iterations
Konstantin Shefov
konstantin.shefov at oracle.com
Wed Dec 10 12:25:21 UTC 2014
On 09.12.2014 16:50, Igor Ignatyev wrote:
> on last thing to think about:
> does it make sense to move 'if (!run.passed) { ... } else { ... }'
> code into TestRun class?
Yes, here is a webrev: http://cr.openjdk.java.net/~kshefov/8066798/webrev.03
>
>
> 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
>>>>>
>>>>
>>
>
More information about the core-libs-dev
mailing list