RFR: 8266182: Automate manual steps listed in the test jdk/sun/security/pkcs12/ParamsTest.java [v4]

[Weijun Wang]

On Mon, 9 Aug 2021 11:06:25 GMT, Abdul Kolarkunnu <akolarkunnu at openjdk.org> wrote:

>> ParamsTest is an interop test between keytool <-> openssl. There are some manual steps listed in jdk/sun/security/pkcs12/params/README to perform after the execution of jtreg execution. So this test is to perform that manual steps.
>
> Abdul Kolarkunnu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8266182: Automate manual steps listed in the test jdk/sun/security/pkcs12/ParamsTest.java

Thanks for the fix. Code looks fine. I'm only concerned on the failure when openssl is not found.

test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 48:

> 46: import java.nio.file.Files;
> 47: import java.nio.file.Path;
> 48: import java.nio.file.Paths;

IntelliJ IDEA shows quite some imports are useless.

test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 88:

> 86:             testWithJavaCommands();
> 87:             testWithOpensslCommands(opensslPath);
> 88:         } else {

I still think it's better to succeed with a warning when the openssl binary cannot be found. IMHO it's a little unfriendly to force people setting up a system property to let the test pass. It's also dependent on runner machines and the user might have to adjust their scripts or launcher all the time. I would rather keep the old test/data if I want to ensure the test gets run everywhere.

Also, it might also help if the test can search for openssl, maybe simply from `/usr/bin/openssl` or `/usr/local/bin/openssl`, but this means you might need to check for the version.

Third, is it OK to let the system property pointing to the binary itself directly? When I was trying out this test I was using the openssl I built and it's not in a `bin` sub-directory.

test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 100:

> 98:         OutputAnalyzer output = ProcessTools.executeCommand(
> 99:                 opensslPath, "version")
> 100:                 .shouldHaveExitValue(0);

This looks like a good time to ensure the version is 1.1.* or at least 1.* (I haven't tested with 1.0.? versions). Also, when trying out with 3.0.0, there are only 2 failures (line 119 generating weak file without -legacy. line 479 succeeds with a warning).

test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 580:

> 578:         return SecurityTools.keytool(s);
> 579:     }
> 580: 

I feel the lines below should go to a test library.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4413