RFR: 8350964: Add an ArtifactResolver.fetch(clazz) method [v2]

Fernando Guallini fguallini at openjdk.org
Wed Mar 12 12:19:59 UTC 2025


On Tue, 11 Mar 2025 16:48:03 GMT, Matthew Donovan <mdonovan at 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


More information about the security-dev mailing list