Code Review Request: More tests for 7184826: (reflect) Add support for Project Lambda concepts in core reflection

Amy Lu amy.lu at oracle.com
Mon Jul 22 13:12:41 UTC 2013


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