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