RFR: 8345057: ML_KEM NamedParameterSpec constants removed by ML-DSA integration

[Sean Mullan]

On Tue, 26 Nov 2024 19:04:47 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

> Could someone please help review this trivial fix?
> 
> The constants for ML-KEM are accidentally removed during merge. This PR adds them back and adds a test for checking them against the expected ones.

A few small comments but this looks fine in order to get this fixed quickly. However, I think it would be better if there were ML-KEM and ML-DSA specific tests that used the public constants. I think we should followup on that with Ben and Rajan and possibly open a separate issue to improve existing tests.

test/jdk/java/security/spec/TestNamedParameterSpec.java line 33:

> 31: import java.lang.reflect.Field;
> 32: import java.lang.reflect.Modifier;
> 33: import java.security.spec.*;

Nit: can you just import `NamedParameterSpec`?

test/jdk/java/security/spec/TestNamedParameterSpec.java line 60:

> 58:     public static String[] getSortedConstNames(Class<?> clazz) {
> 59:         TreeSet<String> names = new TreeSet<String>();
> 60:         for(Field field : clazz.getDeclaredFields()){

Nit: add space after "for".

-------------

Marked as reviewed by mullan (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22397#pullrequestreview-2462492663
PR Review Comment: https://git.openjdk.org/jdk/pull/22397#discussion_r1859113300
PR Review Comment: https://git.openjdk.org/jdk/pull/22397#discussion_r1859111279