On Mon, 13 Apr 2026 18:42:27 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/DKSTest.java line 216:
> 
>> 214:         }
>> 215:         keystore.setEntry(alias,
>> 216:                 new KeyStore.TrustedCertificateEntry(cert), null);
> 
> Is this `setEntry` call here just verifying that no exception is thrown, or 
> should there be a subsequent `isCertificateEntry` check?

already a separate test `keystoreIsCertificateEntryTest`

> test/jdk/sun/security/provider/KeyStore/DKSTest.java line 429:
> 
>> 427:         }
>> 428:         return false;
>> 429:     }
> 
> Maybe it would make sense to rewrite this `causedBy` as custom assertion 
> method, since that is the only way this is currently used?
> 
> 
> // asserts that an exception was caused by specified exception class
> private static void assertCausedBy(Exception e, Class klass) {
>     Throwable cause = e;
>     while ((cause = cause.getCause()) != null) {
>         if (cause.getClass().equals(klass)) {
>             return;
>         }
>     }
>     fail("Exception not caused by " + klass, e);
> }

I personally prefer the older way since it is easier to read to me

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30712#discussion_r3078926949
PR Review Comment: https://git.openjdk.org/jdk/pull/30712#discussion_r3078938579

Reply via email to