On Thu, 16 Jul 2026 12:41:23 GMT, Artur Barashev <[email protected]> wrote:
>> Andreas Chmielewski has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Added visibiliy checks; Reduced spanwed VMs
>> - Shorten spawned VMs
>
> test/jdk/javax/net/ssl/ciphersuites/BulkCipherDisabledAlgorithms.java line
> 113:
>
>> 111: for (CipherSuite suite : suites) {
>> 112: String suiteName = suite.name();
>> 113: String bulkCipher = extractBulkCipher(suiteName);
>
> We can further reduce the number of VMs by grouping the suites by
> `bulkCipher`, i.e. make `tests` a map with `bulkCipher` as a key and a list
> of suites as a value. Then change the rest of the code accordingly. We really
> need only a single VM per a bulk cipher.
Yes, that's certainly possible. However, I would prefer not to take that
approach.
I see this test primarily as a black-box test that iterates over all enabled
cipher suites and verifies that disabling their corresponding bulk cipher
algorithm has the expected effect. The test intentionally does not depend on
the details of the bulk cipher decomposition logic.
Grouping cipher suites by bulk cipher would introduce additional logic and
assumptions into the test itself. It would also make the test more complex,
fragile and potentially reduce coverage in the future. At that point, I think
we would be optimizing the test beyond what is necessary.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31812#discussion_r3595768206
