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

[Tim Du]

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