On Fri, 8 Nov 2024 19:34:42 GMT, Mark Powers <mpow...@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
