RFR: 8342442: Static ACVP sample tests [v12]

[Weijun Wang]

On Fri, 8 Nov 2024 21:39:45 GMT, Sean Mullan <mullan at openjdk.org> wrote:

> Do we have plans to add more tests in the future? I see test data for AES_GCM, HMAC-SHA*, RSA, among others at the github page.

It's definitely better to add more, but that's not in my plan yet. We can discuss with the test team on how to cover more algorithms.

> test/jdk/sun/security/provider/acvp/Launcher.java line 1:
> 
>> 1: /*
> 
> By default, does this test run and do nothing because there are no `internalProjection.json` files? Are those going to be added as test data files later? Should the test be reported as skipped if there is nothing to test?

By default, it's a no op. We will add test data files later.

Personally, I don't think it's worth being reported as skipped.

> test/jdk/sun/security/provider/acvp/Launcher.java line 36:
> 
>> 34:  */
>> 35: 
>> 36: /// This test runs on `internalProjection.json`-style files generated
> 
> Is there a reason you didn't put all these comments (or a copy of them) in the README file instead? It seems like a better place for it. The README doesn't help me understand how to run the tests.

The README file is inside a sub-directory and I created it before adding `acvp.md` so that directory is not empty (Git does not support empty directories). I'd like to keep the text here and remove that README, Or, if you believe a standalone README is better, I can move the README out of `data` and put it here in the same level as this file, and move these text there.

> test/jdk/sun/security/provider/acvp/Launcher.java line 43:
> 
>> 41: /// The test walks through the directory recursively and looks for
>> 42: /// file names equals to or ending with `internalProjection.json` and
>> 43: /// runs test on them.
> 
> s/test/tests/

OK.

> test/jdk/sun/security/provider/acvp/Launcher.java line 45:
> 
>> 43: /// runs test on them.
>> 44: ///
>> 45: /// Set the `test.acvp.alg` test property to only test this algorithm.
> 
> What is "this algorithm"? Should it maybe be "the specified algorithm"?

Correct. Will fix.

> test/jdk/sun/security/provider/acvp/Launcher.java line 51:
> 
>> 49: ///
>> 50: /// By default, the test uses system-preferred implementations.
>> 51: /// If you want to test on a specific provider, set the
> 
> s/on a/a/

OK.

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

PR Comment: https://git.openjdk.org/jdk/pull/21548#issuecomment-2465821736
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835102162
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835100271
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835100569
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835100878
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835101004