Review request for CR 8049039 Need new tests for sun.securiy.x509 classes

[raghu k.nair]

Hi Jason,
   I have removed those lines from copyright headers. Here is the 
updated webrev.
  http://cr.openjdk.java.net/~rhalade/8049039/webrev.04/

Thanks,
Raghu Nair
On 9/6/2014 1:58 AM, Jason Uh wrote:
> Hi Raghu,
>
> Formatting looks good, but one last thing about the copyright headers. 
> I noticed that in lines 7-9, you have the lines
>
>   * ...  Oracle designates this
>   * particular file as subject to the "Classpath" exception as provided
>   * by Oracle in the LICENSE file that accompanied this code.
>
> Can you remove that sentence? In general, I think the license that 
> includes those lines is meant for source files, not for test files. If 
> you look at other tests, you'll see that this final sentence isn't there.
>
> Thanks,
> Jason
>
> On 9/4/14 6:54 AM, raghu k.nair wrote:
>> Hi Jason,
>>   Please review the updated webrev . I have addressed your comments.
>> http://cr.openjdk.java.net/~tyan/raghu/8049039/webrev03/
>> <http://cr.openjdk.java.net/%7Etyan/raghu/8049039/webrev03/>
>>
>> Thanks,
>> Raghu
>> On 9/3/2014 12:47 AM, Jason Uh wrote:
>>> On 8/27/14 8:34 AM, raghu k.nair wrote:
>>>> Hi Vincent / Jason,
>>>>    Please review the updated webrev  based on Jason's comments.
>>>>    webrev: http://cr.openjdk.java.net/~tyan/raghu/8049039/webrev02/
>>>> <http://cr.openjdk.java.net/%7Etyan/raghu/8049039/webrev02/>
>>>>
>>>> Thanks,
>>>> Raghu Nair
>>>> On 8/27/2014 9:43 AM, raghu k.nair wrote:
>>>>> Hi Jason,
>>>>>  Thanks for your review comments.
>>>>> On 8/26/2014 11:37 PM, Jason Uh wrote:
>>>>>> Hi Raghu,
>>>>>>
>>>>>> I'm not an official Reviewer, but I have some comments, mostly about
>>>>>> X509Test.java.
>>>>>>
>>>>>> 1. I'm not sure if this matters, but the formatting of your 
>>>>>> copyright
>>>>>> header is a little strange. The text appears to be correct, but the
>>>>>> line breaks occur at non-standard places. If you look at any other
>>>>>> file, the first couple lines would look like this:
>>>>>>
>>>>>> /*
>>>>>>  * Copyright (c) 2014, Oracle and/or its affiliates. All rights
>>>>>> reserved.
>>>>>>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>>>
>>>>>> Again, not sure how important this is, but it applies to every file
>>>>>> in the webrev. I'd suggest changing them.
>>>>>>
>>>>> This happened due to default formatting by Netbeans . I will correct
>>>>> it.
>>>
>>> I was actually showing the issue with the entire copyright header by
>>> quoting just the beginning. Sorry if I wasn't clear, but it looks like
>>> you only corrected the formatting on the first two lines. Could you
>>> please redo the entire copyright header for all of these files? (You
>>> can look at any existing test for reference and you'll see the
>>> difference.)
>>>
>>>>>> 2. checkMethod():
>>>>>> Doesn't appear to be used anywhere. Do you want to keep this? If you
>>>>>> do, could you add a comment to explain the parameters?
>>>>>>
>>>>>> Also, in general for the rest of the methods, there are a lot of
>>>>>> parameters noted with the @param tag. If you want to leave those
>>>>>> javadoc style comments in, could you please describe the params?
>>>>>>
>>>>> X509Test is used by many other tests ( appart from these tests as 
>>>>> part
>>>>> of the webrev). I will add comment and description for the params.
>>>>>
>>>
>>> Minor detail, but there is an extra space in line 53 and 55 of
>>> X509Test. Could you please remove them?
>>>
>>> Otherwise, looks good to me, but you'll still need an official 
>>> Reviewer.
>>>
>>> Thanks,
>>> Jason
>>>
>>>>>> 3. equals() and assertByteArray() both seem to be
>>>>>> "assertEquals()"-style methods, but they are named a bit 
>>>>>> differently.
>>>>>> could you make the naming consistent? For example, maybe 
>>>>>> assertEquals
>>>>>> / assertByteArrayEquals.
>>>>>>
>>>>> Yes it make sense. I will make the necessary changes .
>>>>>
>>>>> 4. getElementesTest() should probably read getElementsTest(). You use
>>>>> this method in IssuerAlternativeNameExtensionTest.java, by the way.
>>>>>
>>>>> typo needs to be fixed.
>>>>>
>>>>> Thanks,
>>>>> Raghu
>>>>>> Otherwise, I think the tests look good to me, but again, I'm not a
>>>>>> Reviewer, so you still need an official review.
>>>>>>
>>>>>> Thanks,
>>>>>> Jason
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 08/25/2014 03:59 AM, raghu k.nair wrote:
>>>>>>> Hi Vincent / Jason,
>>>>>>>   Could you please help in reviewing the following test.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Raghu Nair
>>>>>>>
>>>>>>> On 8/12/2014 10:28 AM, raghu k.nair wrote:
>>>>>>>> Hello,
>>>>>>>>  Please review the tests for sun.security.x509 classes.
>>>>>>>>  These cover tests for GeneralName, GeneralNames, GeneralSubtree,
>>>>>>>> GeneralSubtrees, IPAddressName and IssuerAlternativeNameExtension.
>>>>>>>>
>>>>>>>> webrev:
>>>>>>>> http://cr.openjdk.java.net/~rhalade/8049039/webrev.00/
>>>>>>>>
>>>>>>>> Bug:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8049039
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Raghu Nair
>>>>>>>
>>>>>
>>>>
>>>
>>
>