On Mon, 9 Aug 2021 13:04:03 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> 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 > > 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. Ok, changed openssl selection flow as below: 1. Check whether test.openssl.path is set and it's the the preferred version(1.1.*) of openssl 2. If above property doesn't set, then look for already installed openssl (version 1.1.*) in system path /usr/bin/openssl or /usr/local/bin/openssl 3. if above is also not available try to download openssl from artifactory If any of above 3 succeeds then perform all tests, otherwise skip all openssl command dependent tests. ------------- PR: https://git.openjdk.java.net/jdk/pull/4413