RFR: 8367096: jdk/open/test/jdk/sun/security/pkcs11/ rsa, ec, config, secmod and sslecc tests are skipping but showing as pass [v2]

[Mikhail Yankelevich]

On Tue, 9 Sep 2025 11:19:06 GMT, Matthew Donovan <mdonovan at openjdk.org> wrote:

>> Mikhail Yankelevich has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Matthew's comments
>
> test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java line 51:
> 
>> 49:             Provider p = Security.getProvider("SunPKCS11");
>> 50:             if (p == null) {
>> 51:                 throw new SkippedException("Skipping test - no PKCS11 provider available");
> 
> minor wording nit: you don't need to say "skipping test" in the message of a SkippedException.

Done in the next commit

> test/jdk/sun/security/pkcs11/PKCS11Test.java line 784:
> 
>> 782:     private void premain(Provider p) throws Exception {
>> 783:         if (skipTest(p)) {
>> 784:             throw new SkippedException("Test is skipped due to skipTest() result, please see the log");
> 
> maybe the message could just say "See logs for details"? Ideally, I think we'd just refactor `skipTest()` into something like `checkSkippedTest()` and the method itself threw the SkippedException but that would require a lot of refactoring.

I agree, this can just say  "See logs for details", should be done in the next commit. 
Refactoring skipTest will cause a change in classes that have methods overwriting this one. I'm not convinced that such a large change would be feasible given that overwritten methods log the errors anyway.

> test/jdk/sun/security/pkcs11/ec/ReadCertificates.java line 82:
> 
>> 80:     public void main(Provider p) throws Exception {
>> 81:         if (p.getService("Signature", "SHA1withECDSA") == null) {
>> 82:             throw new SkippedException("Provider does not support ECDSA, skipping...");
> 
> here (and the other ec/, rsa/, sslecc/ tests), you could remove the "skipping..." from the message.

Done in the next commit

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27166#discussion_r2333459439
PR Review Comment: https://git.openjdk.org/jdk/pull/27166#discussion_r2333452857
PR Review Comment: https://git.openjdk.org/jdk/pull/27166#discussion_r2333458924