On Tue, 11 Mar 2025 15:59:53 GMT, Fernando Guallini <fguall...@openjdk.org>
wrote:
>> Matthew Donovan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Changed exception message in Artifact resolver and fixed logic in keytool
>> test
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23989#discussion_r1989731829
