code review request: 8012971 PKCS11Test hiding exception failures

[Anthony Scarpino]

It was necessary, but I found the unused variable that was triggering 
the compile error without it.

Tony

On 07/18/2013 03:40 PM, Valerie (Yu-Ching) Peng wrote:
>
> Just noticed that the newly-added import statement on line 32 seems
> redundant?
> Webrev seems the same?
> Thanks,
> Valerie
>
> On 07/18/13 15:17, Anthony Scarpino wrote:
>> On 07/18/2013 02:49 PM, Valerie (Yu-Ching) Peng wrote:
>>>
>>> Please find comments inline.
>>>
>>> On 07/17/13 13:51, Anthony Scarpino wrote:
>>>> I have broken these into two webrev.  The first:
>>>>
>>>> JDK-8012971 PKCS11Test hiding exception failures
>>>> http://cr.openjdk.java.net/~ascarpino/8012971/webrev.01/
>>>>
>>>> handles the minimum changes needed for PKCS11Test to function and the
>>>> Problemlist updated to reflect the failures that would show up.
>>> Looks fine in general, I only have some nit comments, and questions
>>> since I have not touched NSS much.
>>> PKCS11Test.java
>>> 1) line 88, change "this finally block" to "any finally block"
>> yep
>>
>>> 2) line 192, add space before and after the "+" for consistency with the
>>> rest of the source, e.g. line 213.
>> yep
>>
>>> 3) The comments from line 58-60 say the default value is "nss3" but the
>>> default value set on line 61 is "softokn3". Seems conflicting?
>>
>> yep.  it nss3 use to be the default, must have forgot the change the
>> comment
>>
>>> 4) Is "nss3" only used for Secmod? It seems that everywhere else you use
>>> the "softokn3" value. If yes, perhaps using "setSecmod()" would be
>>> clearer than "useNSS()".
>>
>> Secmod is the only one currently, but I would rather leave it as NSS
>> because I named it for the library it's looking for.  If another test
>> comes around that is not a Secmod test, then setSecmod() doesn't seem
>> logical.
>>
>> webrev is updated in place.
>>
>>>
>>> Will look at 8020424 later today.
>>> Thanks,
>>> Valerie
>>>>
>>>>
>>>> The second, are the test changes that fix the problems uncovered by
>>>> the above change:
>>>>
>>>> JDK-8020424 The NSS version should be detected before running crypto
>>>> tests
>>>> http://cr.openjdk.java.net/~ascarpino/8020424/webrev.00/
>>>>
>>>> Tony
>>>>
>>>
>>
>