On Fri, 1 Nov 2024 20:06:32 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> rename property names, add an example
>
> test/jdk/sun/security/provider/acvp/Launcher.java line 34:
>
>> 32: * @bug 8342442
>> 33: * @library /test/lib
>> 34: */
>
> This seems more like an '@driver' type of test than the implied `@run` test.
But this is the only main test. I thought `@driver` is used to do some
preparation work for the main test.
> test/jdk/sun/security/provider/acvp/Launcher.java line 45:
>
>> 43: var provProp = System.getProperty("test.acvp.provider");
>> 44: PROVIDER = provProp != null
>> 45: ? Security.getProvider(provProp)
>
> How are errors in the provider prop reported? In a static block, will an
> uncaught exception provide enough/correct information to correct the supplied
> properties.
Good catch. Will throw an exception.
> test/jdk/sun/security/provider/acvp/Launcher.java line 120:
>
>> 118: case "SHA2-256", "SHA2-224", "SHA3-256", "SHA3-224"
>> 119: -> SHA_Test.run(kat, PROVIDER);
>> 120: default -> System.out.println("Skipped unsupported
>> algorithm: " + alg);
>
> This seems more like a test configuration error and should not silently be
> ignored (you have to read the output to see the error/warning).
> Perhaps it could be a "Skipped" kind of test, so it gets reported.
Since there is no test data now, the only way to run this test is to point
`test.acvp.data` to an existing ACVP testsuite data directory and it might
contain tests on other algorithms we don't support yet.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1827885146
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1827887158
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1827881905
