[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

[Igor Ignatyev]

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