On Tue, 11 Mar 2025 15:39:17 GMT, Mikhail Yankelevich
<myankelev...@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
