RFR: 8370942: test/jdk/java/security/Provider/NewInstance.java and /test/jdk/java/security/cert/CertStore/NoLDAP.java may skip without notifying [v2]

[Neha Joshi]

On Mon, 3 Nov 2025 15:39:35 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:

>> Neha Joshi has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   JDK-8370942 : Updated error message
>
> test/jdk/java/security/Provider/NewInstance.java line 30:
> 
>> 28:             JDK JGSS Mechanisms
>> 29:  * @library /test/lib ../..
>> 30:  * @run main/othervm NewInstance
> 
> same as R28 and R29 in NoLDAP.java

Sure

> test/jdk/java/security/Provider/NewInstance.java line 54:
> 
>> 52:             if (p.getName().equals("SunPCSC")) {
>> 53:                 skippedProvider = "SunPCSC";
>> 54:                 System.err.println("Skip test :: A smartcard might not be installed.");
> 
> This case I'd either take SunPCSC from the `Security.getProviders()` (save them to the list, take this out and traverse over it. This way you don't need to put this always skipped message. 
> 
> For SunPCSC, if you think it is needed at all, create a separate `@test` which will take the same providers list and run only if there is `SunPCSC` present and only with it. This way you can have it skipped/failed/passed if it needs to be. 
> 
> What do you think?

Yeah! if the skip case is required only for SunPCSC , then having a diff test case for that should be a better option.

> test/jdk/java/security/cert/CertStore/NoLDAP.java line 29:
> 
>> 27:  *   a CertStore of type "LDAP" and LDAP is not available.
>> 28:  * @library /test/lib ../..
>> 29:  * @run main/othervm NoLDAP
> 
> As you're not changing any env properties here, there is no need for othervm. 
> `@run main NoLDAP` should be sufficient. But you don't need it at all, as `@run main NoLDAP` is there by default, it makes no difference at all.

Sure will update this.

> test/jdk/java/security/cert/CertStore/NoLDAP.java line 44:
> 
>> 42:             throw new SkippedException("Test skipped :: LDAP is present");
>> 43:         } catch (ClassNotFoundException ignore) {
>> 44:             System.err.println("Class not found exception" + ignore.getMessage());
> 
> I'd state that this is expected, otherwise it's a bit confusing. What do you think?

We should never silently ignore exceptions so , it better to log it for making it easier to detect and fix problems.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28112#discussion_r2487078648
PR Review Comment: https://git.openjdk.org/jdk/pull/28112#discussion_r2487064234
PR Review Comment: https://git.openjdk.org/jdk/pull/28112#discussion_r2487077517
PR Review Comment: https://git.openjdk.org/jdk/pull/28112#discussion_r2487076043