RFR: 8342442: Static ACVP sample tests [v12]
Weijun Wang
weijun at openjdk.org
Fri Nov 8 22:07:47 UTC 2024
On Fri, 8 Nov 2024 19:34:42 GMT, Mark Powers <mpowers at openjdk.org> wrote:
>> 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/126f4199...a7e22873
>
> test/jdk/sun/security/provider/acvp/ML_DSA_Test.java line 89:
>
>> 87: s.update(toByteArray(c.get("message").asString()));
>> 88: var sig = s.sign();
>> 89: Asserts.assertEqualsByteArray(sig, toByteArray(c.get("signature").asString()));
>
> If more than one test fails, it might be useful to print out all the failures and then, at the end, throw an assert saying one or more tests failed.
I actually choose your suggested approach while developing on the tests when there were many failed tests. But at the end I think it's good to stop at the failure so it's easier to spot it clearly. Also, our `Asserts` lib does not provide an easy way to work in this way. Maybe we can reconsider this when we have a better infrastructure.
> test/jdk/sun/security/provider/acvp/ML_KEM_Test.java line 60:
>
>> 58: System.out.print(c.get("tcId").asString() + " ");
>> 59: g.initialize(np, new FixedSecureRandom(
>> 60: toByteArray(c.get("d").asString()), toByteArray(c.get("z").asString())));
>
> This line and others below are way over 80 characters.
Same reply as above.
> test/lib/jdk/test/lib/security/FixedSecureRandom.java line 28:
>
>> 26: import java.security.SecureRandom;
>> 27:
>> 28: /// A custom implementation of `SecureRandom` that outputs a
>
> Anything special about `///`?
I'm practicing [JEP 467: Markdown Documentation Comments](https://openjdk.org/jeps/467).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835097315
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835097441
PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835098023
More information about the security-dev
mailing list