RFR: 8349492: Update sun/security/pkcs12/KeytoolOpensslInteropTest.java to use a recent Openssl version [v5]

[Matthew Donovan]

On Wed, 5 Mar 2025 20:05:53 GMT, Fernando Guallini <fguallini at openjdk.org> wrote:

>> test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 78:
>> 
>>> 76:             testWithOpensslCommands(opensslPath);
>>> 77:         } else {
>>> 78:             // since the current version of openssl is not available, skip all
>> 
>> If this test is to check interoperability with OpenSSL, shouldn't we throw a skippedexception or an error instead of just running with java?
>
> Well, the test is also checking with java commands if openssl is available (line 75), then it makes sense to keep it when it is not available as it does not rely on Openssl.

My concern is that a Pass result is ambiguous: we may or may not have verified interoperability with openssl. If the Java portion of the test is valid and tests functionality not covered in other tests then it should be its own test. This test should either run with openssl or throw a SkippedException because openssl is not available.

>> test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java line 79:
>> 
>>> 77:             }
>>> 78:         }
>>> 79:         return verifyOpensslVersion(path, OPENSSL_BUNDLE_VERSION) ? path : null;
>> 
>> Do we want to keep this version check? On the one hand, it ensures that the system binaries or binaries specified via the system property will be a specific, known version, but on the other hand, the only way to run this test with a different version of the library is to change the code. Instead, can we just log the version that is used when the test is run?
>
> I considered removing this check as it seems redundant, but then I realised it was introduced to ensure the bundle version matches the version specified by the artifact name. It's possible that the artifact's filename could show a different version than the actual binaries with `-version`. On the other hand, most tests don't run this extra verification, that could be a good reason to remove it.

I don't have as strong an opinion about this check. If you want to keep it, though, then test really needs to be skipped or failed if the correct version of openssl isn't available. There are a lot of other versions of openssl that I could run this test with and I'd see Pass results when I wasn't actually running openssl at all.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23613#discussion_r1983305018
PR Review Comment: https://git.openjdk.org/jdk/pull/23613#discussion_r1983334180