On Mon, 3 Nov 2025 15:39:35 GMT, Mikhail Yankelevich <[email protected]>
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
