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

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 mlvm-dev mailing list