On Fri, 15 Aug 2025 22:50:31 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> 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. Yes, having tests for those services and their `getInstance(...)` methods makes sense of course, I'm not asking to change those tests. If you re-write `InvalidCryptoDisabledAlgos.java` to directly use the `CryptoAlgorithmConstraints.permit` method, then you can as well move it under `test/jdk/sun/security/util/AlgorithmConstraints` directory. I think it would be a better location for such test than `test/jdk/java/security/Security/SecurityPropFile`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26377#discussion_r2280045286
