On Tue, 23 Sep 2025 15:11:53 GMT, Matthew Donovan <mdono...@openjdk.org> wrote:
>> Mikhail Yankelevich has updated the pull request incrementally with one >> additional commit since the last revision: >> >> renamed id > > test/jdk/sun/security/pkcs11/Signature/KeyAndParamCheckForPSS.java line 98: > >> 96: hashAlg, >> 97: mgfHashAlg); >> 98: skipTest = true; > > should this test also be split into separate @test blocks? There are about 18 > test cases and 17 of them could pass and the last one skipped and the test > gets marked as skipped. There should be no skips in this test with nss, so I don't think this should be an issue here. Splitting this into different `@test` would be though, as it will increase the execution time significantly. > test/jdk/sun/security/pkcs11/Signature/SignatureTestPSS.java line 38: > >> 36: >> 37: /** >> 38: * @test id=old_alg > > For id's, "old_alg" and "new_alg" aren't very descriptive/useful. Maybe > change them to "sha" and "sha3"? Done in the next commit > test/jdk/sun/security/pkcs11/Signature/SignatureTestPSS.java line 79: > >> 77: private static final int UPDATE_TIMES_HUNDRED = 100; >> 78: >> 79: private static boolean skipTest = true; > > It looks like this field is still being and we might test some algorithms but > the still throw a SkippedException. Can you get rid of this uncertainty > somehow? I'm not sure what you mean here. I think the best way would be to split the `@test` and test each algorithm separately. However this approach will take forever to execute, which is a dealbreaker. I can add all the skips into one list and only then print out the skipped parameters, if this helps. However, given that the test is split into sha and sha3, all sha3 will be skipped for now. I'm not sure it would bring much more clarity in this case. What do you think? > test/jdk/sun/security/pkcs11/Signature/SignatureTestPSS2.java line 39: > >> 37: >> 38: /** >> 39: * @test id=old_alg > > use more descriptive id names Done in the next commit ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27367#discussion_r2372711689 PR Review Comment: https://git.openjdk.org/jdk/pull/27367#discussion_r2372704609 PR Review Comment: https://git.openjdk.org/jdk/pull/27367#discussion_r2372698054 PR Review Comment: https://git.openjdk.org/jdk/pull/27367#discussion_r2372702634
