RFR: 8342442: Static ACVP sample tests [v12]
Sean Mullan
mullan at openjdk.org
Fri Nov 8 21:25:47 UTC 2024
On Fri, 8 Nov 2024 18:00:42 GMT, Weijun Wang <weijun at openjdk.org> wrote:
>> Here we have a launcher and several algorithm-specific tests. Users can populate "internalProjection.json" files generated by NIST's ACVP Server into the `data` directory and test them with the launcher.
>>
>> Currently, only SHA2, SHA3, ML-KEM, and ML-DSA are supported.
>
> Weijun Wang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
>
> - Merge branch 'master' into 8342442
> - more precise ML-DSA sigVer negative test
> - more clear exception message
> - check provider availability; SourceRandom in lib; use Utils.toByteArray
> - rename property names, add an example
> - acvp.test.alg system property
> - remove plugin design
> - rename
> - grammar issues
> - license
> - ... and 3 more: https://git.openjdk.org/jdk/compare/d3963867...a7e22873
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?
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.
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/
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"?
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/
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835067732
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835055952
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835056533
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835057439
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835058786
More information about the security-dev
mailing list