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

raghu k.nair raghu.k.nair at oracle.com
Wed Aug 27 04:13:58 UTC 2014


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.
> 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.

> 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
>>




More information about the security-dev mailing list