On Tue, 11 Mar 2025 16:48:03 GMT, Matthew Donovan <mdono...@openjdk.org> wrote:
>> test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 90:
>>
>>> 88: generateInitialKeystores(opensslPath);
>>> 89: testWithJavaCommands();
>>> 90: testWithOpensslCommands(opensslPath);
>>
>> should only `try catch` the Artifact fetching line, as other test methods
>> could potentially throw an IOException and it could get hidden with a
>> SkippedException
>> Suggestion:
>>
>> String opensslPath;
>> try {
>> opensslPath = OpensslArtifactFetcher.getOpensslPath();
>> } catch (IOException exc) {
>> String exMsg = "Can't find the version: "
>> +
>> OpensslArtifactFetcher.getTestOpensslBundleVersion()
>> + " of openssl binary on this machine, please
>> install"
>> + " and set openssl path with property
>> 'test.openssl.path'";
>> throw new SkippedException(exMsg);
>> }
>> // if the current version of openssl is available, perform all
>> // keytool <-> openssl interop tests
>> generateInitialKeystores(opensslPath);
>> testWithJavaCommands();
>> testWithOpensslCommands(opensslPath);
>
> Yep, that makes sense.
looks good thanks
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23989#discussion_r1991370585
