FYI.... Fwd: Re: [9] RFR:JDK-8076359:Test Task: Develop new tests for Leverage CPU Instructions for GHASH and RSA

[Sean Mullan]

Looks good.

Thanks,
Sean

On 11/10/2015 03:36 AM, Tim Du wrote:
> Hi Sean:
>
> Thanks for reviewing the codes. Updated them follow your comments, see
> http://cr.openjdk.java.net/~fyuan/tim/8076359/webrev.02/ ,please help to
> review them again.
>
> Regards
> Tim
> On 11/10/2015 8:18 AM, Sean Mullan wrote:
>> Couple of comments:
>>
>> - SolarisProviderTest is too generic for me. I would call this
>> "PreferredProviderTest". Also, that way it can be enhanced over time
>> if we add preferred providers for other OSes.
>>
>> - In the error messages:
>>
>> s/Get/Got/
>> s/Return/Returned/
>>
>> - Similarly, "SecurityPropertyNegativeTest" is too generic, suggest
>> renaming to "PreferredProviderNegativeTest".
>>
>> s/negativeProvier/negativeProvider
>>
>> --Sean
>>
>>
>>>>> The changes look good to me.  I assume these tests pass?
>>>>>
>>>>> thanks
>>>>>
>>>>> Tony
>>>>>
>>>>> On 11/05/2015 12:22 AM, Tim Du wrote:
>>>>>> Hi Tony:
>>>>>>
>>>>>> Thanks for reviewing the codes.
>>>>>> Updated them follow your suggestion here:
>>>>>> http://cr.openjdk.java.net/~fyuan/tim/8076359/webrev.01/ ,please
>>>>>> help to
>>>>>> review again.
>>>>>>
>>>>>> Regards
>>>>>> Tim
>>>>>> On 11/5/2015 6:19 AM, Anthony Scarpino wrote:
>>>>>>> On 11/03/2015 05:55 PM, Tim Du wrote:
>>>>>>>> Hi All:
>>>>>>>>
>>>>>>>> Please help to review testing Preferred provider configuration
>>>>>>>> feature
>>>>>>>> for JCE .
>>>>>>>>
>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8076359
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8133151
>>>>>>>> webrev: http://cr.openjdk.java.net/~fyuan/tim/8076359/webrev.00/
>>>>>>>>
>>>>>>>> Thank you very much.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Tim
>>>>>>>
>>>>>>> Thanks for the work Tim, your tests cover pretty much everything..
>>>>>>> I've got a few comments, mostly just add-ons or clarifications.
>>>>>>>
>>>>>>>
>>>>>>> 1) I think the best directory for these tests would be
>>>>>>> test/sun/security/jca/.  The jca directory doesn't exist yet, so
>>>>>>> these
>>>>>>> would be the first tests.  My main code change was in
>>>>>>> ProviderList.java which is under the java.base module in
>>>>>>> sun/security/jca/.
>>>>>>>
>>>>>>>
>>>>>>> 2) In SecurityPropertyNegativeTest.java:afterJCESet(), if I
>>>>>>> understand
>>>>>>> the comment and code correctly, I think you want the comment to say:
>>>>>>> /* Test that the setting of the security property after
>>>>>>>    Cipher.getInstance() does not influence previously
>>>>>>>    loaded instances */
>>>>>>>
>>>>>>>
>>>>>>> 3) I think it is valuable to have the failures exceptions to
>>>>>>> print the
>>>>>>> expected provider and the provider that was returned. For example:
>>>>>>>
>>>>>>>   Test Failed: Get wrong provider from Solaris sparcv9 platform
>>>>>>>   Expected provider: SunJCE, Returned provider: OracleUcrypto.
>>>>>>>
>>>>>>>
>>>>>>> 4) In SolarisProviderTest.java, could you add an algorithm test
>>>>>>> after
>>>>>>> line 93?  I think it is useful to verify an algorithm that is not in
>>>>>>> the preferred list is not being redirected.  For example:
>>>>>>>
>>>>>>>   MessageDigest md = MessageDigest.getInstance("MD5");
>>>>>>>   if (!md.getProvider().getName().equals("OracleUcrypto")) {
>>>>>>>       throw new RuntimeException(...);
>>>>>>>   }
>>>>>>>
>>>>>>>
>>>>>>> thanks
>>>>>>>
>>>>>>> Tony
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>