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

Matthew Donovan mdonovan at openjdk.org
Tue Mar 11 16:53:18 UTC 2025


On Tue, 11 Mar 2025 15:39:17 GMT, Mikhail Yankelevich <myankelevich at 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/provider/acvp/Launcher.java line 181:
> 
>> 179:     }
>> 180: 
>> 181:     private static Path fetchACVPServerTests(Class<?> clazz) {
> 
> Is there a point in this method? It's used in 1 spot only it seems and you can just directly call `fetchOne`

It encapsulates all of the logic involved in getting the tests. Specifically, what to do if the tests can't be fetched. It could be done in `main()` but this is a little cleaner.

> test/lib/jdk/test/lib/artifacts/ArtifactResolver.java line 100:
> 
>> 98:             Throwable cause = e.getCause();
>> 99:             if (cause == null) {
>> 100:                 // if property doesn't exist
> 
> As per our discussion, do you think doing it in a way similar to [this](https://github.com/openjdk/jdk/pull/23988/files#diff-65002682c363b32a2f5bc860ec7482f3682cceb2d2c748e5b9d3aa4aedf88d35R66-R72) would be easier to read during a debug?

I updated the message.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23989#discussion_r1989684931
PR Review Comment: https://git.openjdk.org/jdk/pull/23989#discussion_r1989733346


More information about the security-dev mailing list