On Mon, 13 Apr 2026 18:24:37 GMT, Marcono1234 <[email protected]> wrote:
>> * Improving the coverage of the `DomainKeyStore.java` >> * converting existing tests to junit in order to make logs easier to read >> * Further coverage for `DomainKeyStore.java` would be required, but should >> be done as a separate [ticket](https://bugs.openjdk.org/browse/JDK-8382096) >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > test/jdk/sun/security/provider/KeyStore/DKSSystemPropertiesTest.java line 73: > >> 71: keystore.load( >> 72: new DomainLoadStoreParameter(config, >> 73: Collections.<String, >> KeyStore.ProtectionParameter>emptyMap())); > > Are these type args needed? > > Suggestion: > > Collections.emptyMap())); I'd prefer to keep it in case of a backport. This test didn't have any logic changes, so should remain straight forward > test/jdk/sun/security/provider/KeyStore/DKSSystemPropertiesTest.java line 94: > >> 92: Date.from(keystore.getCreationInstant(alias))) >> 93: ) { >> 94: throw new RuntimeException( > > For consistency with the other exception thrown in this method, can this > simply throw `Exception` as well? > > Suggestion: > > throw new Exception( Generally for test failures we are using `RuntimeException` since they are happening during the runtime. I'd actually prefer to change the other exception to `RuntimeException` > test/jdk/sun/security/provider/KeyStore/DKSTest.java line 29: > >> 27: * @library /test/lib >> 28: * @summary Support the logical grouping of keystores >> 29: * @run junit DKSTest > > In some of the other JUnit PRs it was recommended to use `${test.main.class}` > to avoid repetition. Would that work here as well? > > Suggestion: > > * @run junit ${test.main.class} I'd prefer to keep explicit name as this is the format generally used by security tests ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30712#discussion_r3078490110 PR Review Comment: https://git.openjdk.org/jdk/pull/30712#discussion_r3078499316 PR Review Comment: https://git.openjdk.org/jdk/pull/30712#discussion_r3078505664
