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