On Thu, 4 Sep 2025 14:09:54 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Mikhail Yankelevich has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> JDK-8366159: SkippedException is treated as a pass for pkcs11/KeyStore,
>> pkcs11/SecretKeyFactory and pkcs11/SecureRandom
>
> 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");
>
> The original test runs all cases but the updated would stop once any of them
> is skipped. Is this intended?
>
> Also, I cannot see a benefit to rethrow a `SkippedException` with only a
> modified message. Why not just let it bubble up?
Thank you for noticing, this is definitely not intentional. It should show this
error only if there are some tests skipped, but should still run all of them.
This way the error will not be hidden by the skip in another test.
This is also why I didn't bubble it up, as this was not supposed to be
triggered until all the tests are finished.
This should be fixed in the next commit
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26942#discussion_r2322443827
