On Thu, 30 Oct 2025 16:59:21 GMT, Matthew Donovan <[email protected]> wrote:
>> Weijun Wang has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains five commits:
>>
>> - Merge branch 'master' into 8349732
>> - rfc; test
>> - some rename
>> - Merge branch 'master' into 8349732
>> - the fix
>
> test/lib/jdk/test/lib/security/DataFetcher.java line 59:
>
>> 57: /// @param zipPrefix the common prefix for each entry in the ZIP file
>> 58: /// @param entry the entry name without `zipPrefix`
>> 59: public static byte[] fetchData(Class<?> klass, String zipPrefix,
>> String entry)
>
> Should this method be in `ArtifactResolver` instead or maybe just in the test
> itself? I don't see a general use for this based on other tests that use
> external "artifacts."
If we have more similar ZIP bundles from GitHub repos, it could be general. I
don't want to put it in `ArtifactResolver` yet.
> test/lib/jdk/test/lib/security/DataFetcher.java line 78:
>
>> 76: } else {
>> 77: try {
>> 78: Path p =
>> ArtifactResolver.resolve(klass).entrySet().stream()
>
> You don't need to introduce a new property. `ArtifactResolver.resolve()`
> already looks for a System property of the form `jdk.test.lib.artifacts.NAME`
> (where NAME = the name from the @Artifact class). If the property is
> specified, its value is returned from `resolve()` unmodified. You can get the
> URL from there.
Yes, but that means I need to download the ZIP bundles somewhere. This new
property allows me to work with local or remote repositories directly. Maybe
not many people will use it.
> test/lib/jdk/test/lib/security/DataFetcher.java line 101:
>
>> 99: extension = "zip",
>> 100: unpack = false)
>> 101: public static class DILITHIUM_CERTIFICATES {
>
> I think these should be defined in the tests where they are used.
Soon, `DILITHIUM_CERTIFICATES` will be used by KAT for RFC 9881 and whatever
[draft-ietf-lamps-kyber-certificates](https://datatracker.ietf.org/doc/draft-ietf-lamps-kyber-certificates/)
will be. We can revise the structure later.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26563#discussion_r2479018342
PR Review Comment: https://git.openjdk.org/jdk/pull/26563#discussion_r2479015653
PR Review Comment: https://git.openjdk.org/jdk/pull/26563#discussion_r2479021588
