RFR(S) : 8038186 : [TESTBUG] improvements of test j.l.i.MethodHandles (original) (raw)

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Fri Mar 28 09:29:29 UTC 2014

Igor, thanks for improving the tests on method handles!

After an offline conversation w/ VladimirI, I've done several changes: - placed all classes into one file (but I don't like it) You can convert TestCase & TestFactory to inner classes and move ThrowMode into TestCase since it's usage is confined to TestCase.

- changed package name - extracted creating tests code from test execution code These changes look good.

Other comments: - why don't you remove TestCase.assertEQ and enhance Asserts.assertEquals for arrays?

- regarding CatchExceptionTest name: there's already

TestCatchException test. Have you considered merging them into one test or removing TestCatchException, if your test already covers that cases?

Best regards, Vladimir Ivanov

PS: John mentioned recently that there's a convention for java/lang/invoke tests to be in JUnit format. I'm not sure it'll improve the code in any way (maybe TestNG will?). I hope John will express his opinion on this, if it's important.

I hope the code's still quite readable and understandable. Also I've added array return type. http://cr.openjdk.java.net/~iignatyev/8038186/webrev.02/ Thanks, Igor On 03/27/2014 12:46 AM, Igor Ignatyev wrote: Hi Chris,

This is very nice. Are there plans to do this for the other existing tests as well? sure, but we didn't have enough time to do it in 8u20 timeframe. So I'd like to push these changes 1st, since we need this to cover 'Speedup catch exceptions'. Other tests will be rewrite later. I hope we'll have time in jdk 9 to refactor all tests in MethodHandles. Thanks, Igor On 03/27/2014 12:38 AM, Christian Thalinger wrote:

On Mar 24, 2014, at 1:33 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:

Hi all,

Please review the patch: Problems: - MethodHandlesTest::testCatchException() doesn't provide enough testing of j.l.i.MethodHandles::catchException(). - MethodHandlesTest contains more than 3k lines, an auxiliary code together w/ a test code, many methods aren't connected w/ each other, etc. All this leads to that it is very very hard to understand, maintain. Yes, it is.

Fix: - the auxiliary code was moved to testlibray - the test code was moved to a separate subpackage This is very nice. Are there plans to do this for the other existing tests as well? - random signature is used for target and handler - added scenarios w/ different return types webrev: http://cr.openjdk.java.net/~iignatyev/8038186/webrev.01/ jbs: https://bugs.openjdk.java.net/browse/JDK-8038186 -- Igor

mlvm-dev mailing list mlvm-dev at openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

mlvm-dev mailing list mlvm-dev at openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

mlvm-dev mailing list mlvm-dev at openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

mlvm-dev mailing list mlvm-dev at openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

More information about the core-libs-dev mailing list