On Tue, 26 Aug 2025 12:45:46 GMT, Francesco Andreuzzi <d...@openjdk.org> wrote:
>> Skipped tests are treated as a pass:
>>
>> * test/jdk/sun/security/pkcs11/KeyStore/ClientAuth.java
>> * test/jdk/sun/security/pkcs11/KeyStore/CertChainRemoval.java
>> * test/jdk/sun/security/pkcs11/SecretKeyFactory/TestGeneral.java
>> * test/jdk/sun/security/pkcs11/SecureRandom/Basic.java
>> * test/jdk/sun/security/pkcs11/SecureRandom/TestDeserialization.java
>
> test/jdk/sun/security/pkcs11/KeyStore/CertChainRemoval.java line 130:
>
>> 128: printKeyStore("Initial PKCS11 KeyStore: ", p11ks);
>> 129: } catch (Exception e) {
>> 130: throw new SkippedException("Skip test, due to " + e);
>
> Maybe `e` can be specified as a real `cause` now?
I might be misunderstanding your point, but I think in this case a full
exception stack trace would be more helpful. It covers everything this way.
> test/jdk/sun/security/pkcs11/SecretKeyFactory/TestGeneral.java line 125:
>
>> 123: test("Blowfish", bf_128Key, p, TestResult.PASS);
>> 124: } catch (SkippedException skippedException){
>> 125: throw new SkippedException("One or more tests skipped");
>
> I'd attach `skippedException` as the `cause` here
Skips are logged above for each of the tests. I wanted to avoid changing the
logic if possible.
> test/jdk/sun/security/pkcs11/SecureRandom/Basic.java line 50:
>
>> 48: } catch (NoSuchAlgorithmException e) {
>> 49: e.printStackTrace();
>> 50: throw new SkippedException("Provider " + p + " does not
>> support SecureRandom, skipping");
>
> If you add `e` as the cause here, you can probably get rid of the explicit
> `e.printStackTrace()`
It is set there on purpose. This way it will log to the `System.out` and not
`System.err`. Just easier to spot this way. I can add `e` into skipped
exception, but it seems to be a bit of an overkill. What do you think?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26942#discussion_r2301160109
PR Review Comment: https://git.openjdk.org/jdk/pull/26942#discussion_r2301149313
PR Review Comment: https://git.openjdk.org/jdk/pull/26942#discussion_r2301154694
