On Mon, 11 Aug 2025 15:34:31 GMT, Artur Barashev <abaras...@openjdk.org> wrote:
>> Valerie Peng has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address review comments from Artur and updated tests to leverage >> Utils.runAndCheckException > > test/jdk/java/security/Security/SecurityPropFile/InvalidCryptoDisabledAlgos.java > line 1: > >> 1: /* > > It would be nice to also have a dedicated test class under > `sun/security/utils` that tests everything directly, including invalid values > for the `permit` call. See `DisabledAlgorithmPermits` class for example. Hmm, I looked at that test as well as the changes that it corresponds to. Given that this PR involves public JCA classes, e.g. the 4 services, having tests for those services and their `getInstance(...)` methods makes more sense as we need to ensure that the javadoc `@implNote` matches the actual behavior. Testing the `permit` call makes a lot sense for checking invalid values though. So, I will explore re-writing the `test/jdk/java/security/Security/SecurityPropFile/InvalidCryptoDisabledAlgos.java` to directly using the `CryptoAlgorithmConstraints.permit` method. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26377#discussion_r2279998547
