On Tue, 3 Feb 2026 19:32:35 GMT, Weijun Wang <[email protected]> wrote:
>> There are new test cases inside the ACVP test for ML-KEM.
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> refactoring structures
Just a few questions
test/jdk/sun/security/provider/acvp/Launcher.java line 2:
> 1: /*
> 2: * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights
> reserved.
nit: copyright
test/jdk/sun/security/provider/acvp/ML_KEM_Test.java line 30:
> 28:
> 29: import javax.crypto.KEM;
> 30: import java.security.*;
Minor: could you please remove wildcard import?
test/jdk/sun/security/provider/acvp/ML_KEM_Test.java line 91:
> 89: var function = t.get("function").asString();
> 90: System.out.println(">> " + pname + " " + function);
> 91: for (var c : t.get("tests").asArray()) {
Minor: could you please change this var to a class name? It's a bit hard to
read IMO
test/jdk/sun/security/provider/acvp/ML_KEM_Test.java line 112:
> 110: g.newEncapsulator(ek);
> 111: } else {
> 112: Asserts.assertThrows(Exception.class, ()
> -> g.newEncapsulator(ek));
Do you think it's best to change this to expect a more exact exception? Unless
I'm mistaken the method `newEncapsulator` throws InvalidKeyException. This way
the test won't accept an unexpected null pointer for example
test/jdk/sun/security/provider/acvp/ML_KEM_Test.java line 124:
> 122: if (function.equals("decapsulation")) {
> 123: var d = g.newDecapsulator(dk);
> 124: var k =
> d.decapsulate(toByteArray(c.get("c").asString()));
minor: the same with `k` as above, without being familiar with the test it's
quite hard to read.
test/jdk/sun/security/provider/acvp/ML_KEM_Test.java line 130:
> 128: g.newDecapsulator(dk);
> 129: } else {
> 130: Asserts.assertThrows(Exception.class, ()
> -> g.newDecapsulator(dk));
The same question as above on line 112 :)
-------------
PR Review: https://git.openjdk.org/jdk/pull/29548#pullrequestreview-3750168100
PR Review Comment: https://git.openjdk.org/jdk/pull/29548#discussion_r2763230789
PR Review Comment: https://git.openjdk.org/jdk/pull/29548#discussion_r2763262957
PR Review Comment: https://git.openjdk.org/jdk/pull/29548#discussion_r2763229365
PR Review Comment: https://git.openjdk.org/jdk/pull/29548#discussion_r2763256430
PR Review Comment: https://git.openjdk.org/jdk/pull/29548#discussion_r2763238543
PR Review Comment: https://git.openjdk.org/jdk/pull/29548#discussion_r2763258871
