Review request for type annotation reflection test

Jonathan Gibbons jonathan.gibbons at oracle.com
Tue May 21 12:49:06 PDT 2013


Charlie,

It looks like your test files are probably missing the final newline, 
judging by the apparent lack of the final '}'

I would not advise the use of lots of top level classes and lots of 
tests in the same directory, especially when many of the top level 
classes have the same name.

Stylistically, beginning a line with "," is unusual, Normally, a comma 
is placed at the end of the preceding expression.

There are indentation issues, such as in the first file, lines 60-62.

GetAnnotatedInterfacesTest.java, 64: just because the exception is 
expected for TypeAnno3 doesn't mean you should ignore it for all other 
test cases, does it?

GetAnnotationsInterfacesTest has a lot of low level hacking, modelling 
info in arrays of objects, and with a lot of index calculation, like [j* 
2 + 1]. Generally, I'd say this is indication of missing abstractions.

-- Jon

On 05/21/2013 03:10 AM, Charlie Wang wrote:
> Yes, that make sense. What do you think if I combine the tests? It 
> would then have 1 unified data provider and test method, thus solve 
> the 2 problems.
>
> Regards,
> Charlie
> On 2013/5/21 14:56, Werner Dietl wrote:
>> I agree with Alex's first two comments. The code duplication for the
>> test logic and all the different annotation types seems hard to
>> maintain and update.
>>
>> cu, WMD.
>>
>> On Mon, May 20, 2013 at 7:56 PM, Charlie Wang 
>> <charlie.wang at oracle.com> wrote:
>>> Hi,
>>>    Here's the latest one with a little update:
>>> http://cr.openjdk.java.net/~ssides/8013497/webrev.03/
>>>    Please provide comments as soon as possible now that due date is
>>> approaching.
>>>
>>>
>>> Regards,
>>> Charlie
>>>
>>>
>>>
>>> On 2013/5/19 14:10, Charlie Wang wrote:
>>>
>>> Hi,
>>>    Here's test for core reflection support of type-annotations 
>>> JDK-8013497.
>>> Please take a look.
>>>
>>> http://cr.openjdk.java.net/~ssides/8013497/webrev.02/
>>>
>>>
>>> Regards,
>>> Charlie
>>>
>>>
>>>
>>>
>>
>>
>



More information about the type-annotations-dev mailing list