Review request for type annotation reflection test

Alex Buckley alex.buckley at oracle.com
Tue May 21 11:24:36 PDT 2013


Please keep one source file per tested API method, because it is easy to 
navigate. The problem is the body of the test() method in each source 
file. It should call utility methods to inspect annotations, rather that 
repeating more or less the same 'for' loops as every other test() method.

As for data providers, I believe they are a TestNG thing. Please add 
comments to each source file to describe what the data provider represents.

Finally, when you make a "little update", you should give a short 
description of what has changed. If you don't, someone who looked at the 
previous version has no idea what changed and will have to look at 
everything again.

Alex

On 5/21/2013 3: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