Review request for type annotation reflection test

Jonathan Gibbons jonathan.gibbons at oracle.com
Tue May 21 11:46:42 PDT 2013


These tests contain a curious line in the test description, such as the 
following.

   27  * @(#) GetAnnotatedInterfacesTest.java


Last I heard, we stopped using SCCS when we converted to using Mercurial 
back in 2007.   I'm not sure where you're getting your template from but 
these lines can/should be deleted.

-- Jon

On 05/21/2013 11:24 AM, Alex Buckley wrote:
> 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