Hi Amanda,
Thanks for the update. Few more comments after review:
- TestSameLength.java: line 93-103, check here should use provider used
with test instance of MessageDigest, call md.getProvider(). You will
need to reorganize test code for this. Security.getProvider() returns
provider installed with specified name.
- It will be good if you have a function isSHA3Supported() to check SHA3
supported or not
Same comments applies from other MessageDigest tests in your review.
Thanks,
Rajan
On 5/23/16 3:24 PM, Amanda Jiang wrote:
Hi Valerie , Rajan,
Thanks for reviewing the webrev. I have updaed
UnsupportedProvider.java to check NSAE for SUN and OracleUcrypto
providers.
Other tests has been updated by Rajan's comments as well.
Please check the latest webrev and let me know if there are any other
problems.
http://cr.openjdk.java.net/~amjiang/8156059/webrev.02/
Thanks,
Amanda
On 16/5/19 下午3:31, Valerie Peng wrote:
True, I think the UnsupportedProvider.java should assume SUN provider
to support SHA-3 and not allow NSAE.
OracleUcrypto provider only supports SHA-3 on Solaris 12 or later, so
what you have is fine.
As for other tests, as long as SHA-3 algos are covered, it's up to
you to make the enhancements that Rajan suggested.
Thanks,
Valerie
On 5/19/2016 3:18 PM, Rajan Halade wrote:
Hi Amanda,
I have following comments:
- UnsupportedProvider.java line 55: this assumes that it is okay to
have NoSuchAlgorithmException for all providers.
- Update MessageDigest tests to use RandomFactory from
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/f8a3c1510f95/test/lib/testlibrary/jdk/testlibrary.
It logs seed information which will be helpful to reproduce failure.
- Also, MessageDigest tests have no provider checks.
Rest of tests look fine to me. Note, I am not a official reviewer so
please wait for official reviewer to integrate changes.
Thanks,
Rajan
On 5/19/16 11:34 AM, Amanda Jiang wrote:
Hi All,
Please help to review tests for JEP287: SHA-3 Hash Alogrithms.
BugID: https://bugs.openjdk.java.net/browse/JDK-8156059
Webrev: http://cr.openjdk.java.net/~amjiang/8156059/webrev.01/
Test Plan: https://bugs.openjdk.java.net/browse/JDK-8068855
Thanks,
Amanda
