On Wed, 24 May 2023 07:02:55 GMT, Sibabrata Sahoo <ssa...@openjdk.org> wrote:
> Additional Tests for KEM API. test/jdk/javax/crypto/KEM/GenLargeNumberOfKeys.java line 1: > 1: /* 1. `testXDH` and `testEC` are mostly identical. Maybe you can write a single method with 2 extra arguments. 2. According to the spec, multiple keys generated from a *single* `Encapsulator` are different. The `test` method is creating a new encapsulators each time. 3. There is no guarantee that a `SecretKey` follows the `hashCode/equals` convention and can be put inside a `Set` to detect duplication. It happens that in this implementation the key is a `SecretKeySpec` object so it works. test/jdk/javax/crypto/KEM/KemInterop.java line 77: > 75: KEM.Encapsulated enc2 = encT1.encapsulate(); > 76: > 77: Asserts.assertEQ(enc.key(), enc.key()); Again, we cannot guarantee `equals` between 2 `SecretKey` objects. However, since it's a positive test here, it's OK to do this. If we really modify the implementation later and return a different kind of `SecretKey`, we can update the test later. test/jdk/javax/crypto/KEM/KemInterop.java line 81: > 79: Asserts.assertTrue(Arrays.equals(enc.encapsulation(), > enc.encapsulation())); > 80: > 81: Asserts.assertNE(enc.key(), enc1.key()); This is a negative test and we should rely on `!equals` here. I think we can drop this check. If the `enc.key()` check below already shows they are different, then the key will be different too. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1204090836 PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1204119149 PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1204120540
