code review request: 8012971 PKCS11Test hiding exception failures

[Anthony Scarpino]

Both of them get removed in the next webrev.

Tony

On 07/19/2013 04:21 PM, Valerie (Yu-Ching) Peng wrote:
>
> Oh, another thing, I noticed that for ProblemList.txt, line 261 is a
> duplicate of line 289.
> Otherwise, looks fine.
>
> BTW, I didn't get to finish reviewing your second fix today, may take a
> few more days to get that done...
> Thanks,
> Valerie
>
> On 07/19/13 11:19, Anthony Scarpino wrote:
>> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>