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

[Igor Ignatyev]

Vladimir, thanks for review, please my comments inline.

On 03/28/2014 01:29 PM, Vladimir Ivanov wrote:
> 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.
>
I don't like using of inner classes, from my point of view, it makes 
code less readable. But if you insists, I can do it.

>> - 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?
I'm not sure that this doesn't break the other tests which use 
'Asserts.assertEquals', and since I want to push this patch to 8u20, I 
consider it's too risky to change now.

I'll do it in my next improvements of MH tests in jdk9 TF.

>     - 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?
I didn't remove TestCatchException, since I'm not pretty sure that all 
cases are covered and don't have enough time to check it, also this 
test's a regression test which cover some particular test cases.

I plan to add the test cases from TestCatchException to 
'MANDATORY_TEST_CASES' in CatchExceptionTest and remove 
TestCatchException in jdk9 TF.

> 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
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev