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

Reply via email to