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