[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 igor.ignatyev at oracle.com
Tue Dec 9 09:28:44 UTC 2014


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



More information about the core-libs-dev mailing list