Code Review Request: More tests for 7184826: (reflect) Add support for Project Lambda concepts in core reflection
Joseph Darcy
joe.darcy at oracle.com
Mon Aug 5 23:44:49 UTC 2013
Hello,
Sorry for the repeated review delays.
This looks fine to go back.
Thanks,
-Joe
On 7/22/2013 6:27 AM, Joel Borggren-Franck wrote:
> Hi Amy,
>
> I'm happy with the current iteration. I'll help you find an official
> reviewer.
>
> cheers
> /Joel
>
> On 2013-07-22, Amy Lu wrote:
>> Thank you Joel for all the valuable feedback.
>>
>> Test have been refactored and here comes the updated version:
>> https://dl.dropboxusercontent.com/u/5812451/yl153753/7184826/webrev.01/index.html
>>
>> Thanks,
>> Amy
>>
>>
>> On 7/10/13 10:17 PM, Joel Borggren-Franck wrote:
>>> Hi Amy, Tristan,
>>>
>>> I'm not a Reviewer kind of reviewer, but I've started to look at the
>>> code and can sponsor this.
>>>
>>> Some comments on test/java/lang/reflect/Method/invoke/DefaultStaticTest.java:
>>>
>>> As there are a lot of non-public top-level classes perhaps this test
>>> should be in it own directory.
>>>
>>> It is very hard to read the data table:
>>>
>>> 292 {interface1.class, 1, 1, new Object1(), new Object[]{},
>>> 293 new Object[]{}, "interface1", null},
>>>
>>> I believe you should move the methodsNum constant and the declMethods
>>> num constant to an annotation on the interface/class in question. For
>>> example:
>>>
>>> @MyTestData(numMethods=1, declMethods=1)
>>> 41 interface interface1 {
>>> 42 @DefaultMethod(isDefault = true)
>>> 43 @StaticMethod(isStatic = false)
>>> 44 @ReturnValue(value = "interface1.defaultMethod")
>>> 45 default String defaultMethod(){ return "interface1.defaultMethod"; };
>>> 46 }
>>>
>>> That way it is much easier to inspect that the constants are right.
>>>
>>> The same can probably be done with the return values encoded.
>>>
>>> Instead of all these "new Objects[] {}" can't you create a field,
>>>
>>> Object [] EMPTY_PARAMETERS = new Object() {}
>>>
>>> and reuse it?
>>>
>>> That way it will be much easier to verify that the encoded test data is
>>> correct.
>>>
>>> I'll comment on the other tests shortly.
>>>
>>> cheers
>>> /Joel
>>>
>>> On 2013-06-13, Amy Lu wrote:
>>>> This has been pending on review for long ... Please help to review.
>>>> I also need a sponsor for this.
>>>>
>>>> Thank you very much.
>>>>
>>>> /Amy
>>>>
>>>> On 5/23/13 10:48 PM, Amy Lu wrote:
>>>>> Please help to review:
>>>>>
>>>>> More tests for 7184826: (reflect) Add support for Project Lambda
>>>>> concepts in core reflection
>>>>>
>>>>> This includes:
>>>>> 1. improvement for existing tests with more situations
>>>>> 2. Test for invoking interface default/static method by j.l.r.Method
>>>>> 3. Test for invoking interface default/static method by MethodHandle
>>>>>
>>>>> https://dl.dropboxusercontent.com/u/5812451/yl153753/7184826/webrev.00/index.html
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Amy
More information about the core-libs-dev
mailing list