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

[Jason Uh]

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.

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?

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.

4. getElementesTest() should probably read getElementsTest(). You use 
this method in IssuerAlternativeNameExtensionTest.java, by the way.

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
>