On Wed, 2 Aug 2023 22:27:05 GMT, Rajan Halade <rhal...@openjdk.org> wrote:
>> I have updated PKCS11Test.java to mark test as skipped only when all >> testDefault, testNSS, and testDeimos tests are skipped. This file also >> includes new trace messages, code cleanup and format change. Some other test >> files are updated to mark as skipped as they use TestNG framework to execute. >> >> Enhancement [JDK-8313575](https://bugs.openjdk.org/browse/JDK-8313575) is >> filed to consider refractor of tests to split these for us to better track >> the coverage. > > Rajan Halade has updated the pull request incrementally with one additional > commit since the last revision: > > 8313206: revert skipTest update to address with new bug test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java line 70: > 68: main(new SecretKeysBasic()); > 69: } catch (SkippedException se) { > 70: throw new SkipException("One or more tests are skipped"); SkipException is only thrown when all cases skipped in PKCS11Test.main() Line: 215-218. Individual test case skip process are handled with print and flag set in PKCS11Test.main() Line:191-213. So it is not thrown. If it is handled in base class PKCS11Test.main() then these changes in each Test not required. test/jdk/sun/security/pkcs11/PKCS11Test.java line 215: > 213: } > 214: > 215: if (skippedDefault && skippedNSS && skippedDeimos) { Should this be || instead of && and the exception message to be "one or more test case skipped" test/jdk/sun/security/pkcs11/PKCS11Test.java line 277: > 275: Provider[] providers = Security.getProviders(); > 276: for (Provider p : providers) { > 277: if (p.getName().startsWith("SunPKCS11-")) { Please correct me, if i am wrong. But as per my understanding there can be many PKCS11 provider instance exist in same time based on different token configuration and the order can be different too. In that case specifying the default PKCS11 provider could be better which is 'p.getName().equals("SunPKCS11")'. Here it is safe because testDefault() is the 1st method to execute. Also the loop should break immediately after finding the default provider. test/jdk/sun/security/pkcs11/Secmod/TrustAnchors.java line 49: > 47: > 48: public static void main(String[] args) throws Exception { > 49: if (!initSecmod()) { 'throw new SkippedException' could be defined in SecmodTest.initSecmod() method to avoid all these Tests to take care separately. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15125#discussion_r1282718003 PR Review Comment: https://git.openjdk.org/jdk/pull/15125#discussion_r1282719721 PR Review Comment: https://git.openjdk.org/jdk/pull/15125#discussion_r1282773066 PR Review Comment: https://git.openjdk.org/jdk/pull/15125#discussion_r1282706705
