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
