On 4/9/18 10:13 AM, Sibabrata Sahoo wrote:
Here is the new patch addressing all comments.
Webrev:http://cr.openjdk.java.net/~ssahoo/8184359/webrev.01/

Changes includes:
1)
KeyAgreementTest.java
KeySizeTest.java
I have ensured no duplicate Test cases exist for the functionality provided in these 2 files. Due to that I have remove 3 files, DHGenSecretKey.java, SupportedDHKeys.java, UnsupportedDHKeys.java from "open/test/jdk/com/sun/crypto/provider/KeyAgreement" folder.

For DHGenSecretKey, can you add the bugid of 4936763 to the test file in this webrev where the changes were merged and also link the bugs together in JBS? That way we know that these tests are now covering the fixes for that bug. For the others, see below.

Statements which affected due to merging the missing code from these deleted files are 
KeyAgreementTest.java[29, 141], KeySizeTest.java[91-100, 112-162]. Also I have observed that 
KeySizeTest.java Test takes around 2 minute to complete in SOME PLATFORM to complete all 14 
@run and approximately 20 seconds for higher keys > 4096 for "DiffieHellman" 
only after merge. Please suggest if removing the higher Keys from the Test cases will help 
or 20-30 seconds for these higher keys is accepted here?

It's a bit concerning. One way to make the tests run faster is to split them up into separate tests so that they can be run concurrently by jtreg. So we might be making things worse by merging them :(

So I would suggest just merging DHGenSecretKey and leaving the other tests as-is.

2) I have updated the comment in KeyAgreementTest.java[129] to indicate 
provider search order.

Ok.

--Sean

As per Adam's comments the following changes,
3) KeySizeTest.java & NegativeTest.java, which need utility methods from 
Convert.java are using the library instead of defining it's own.
4) MultiThreadTest.java generates two key pairs and do two key agreement 
operations.

Reply via email to