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

Igor, I changed to printf and indent:

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

-Konstantin

On 10.12.2014 16:39, Igor Ignatyev wrote:
>>  147                     testCounter++;
>>  148                 }
>
>>  155             throw new Error(String.format("%d of %d test cases 
>> FAILED! %n"
>>  156                     + "Rerun the test with the same \"-Dseed=\" 
>> option as in the log file!",
>>  157                     failCounter, testCounter));
> incorrect indent
>
>>  System.err.println(String.format("All %d test cases PASSED!", 
>> testCounter));
> PrintStream::printf
>
> otherwise LGTM
>
> Thanks,
> Igor
>
> On 12/10/2014 03:25 PM, Konstantin Shefov wrote:
>>
>> 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
>>>>>>>
>>>>>>
>>>>
>>>
>>