On Thu, 15 Jun 2023 12:33:45 GMT, Matthew Donovan <mdono...@openjdk.org> wrote:
>> Yes, this is what I was thinking. There is another test
>> `test/jdk/javax/net/ssl/ciphersuites/DisabledAlgorithms.java` that is very
>> similar to this test. Can you compare these two tests and consider removing
>> that test, or combining them if there are other things that test is testing
>> that this one isn't?
>
> The tests look similar but are definitely testing different code paths. I
> didn't combine the tests because they're different enough in how they work
> that making them one test would be messier than having two tests. I did,
> however, relocate my new test to the same directory as `DisabledAlgorithms`
> and changed it to use the same list of disabled ciphers so when we need to
> add to the list, it's covered by both tests. Also, while there, I updated
> DisabledAlgorithms to use the SSL test template classes instead of the binary
> {key,trust}store files.
Ok, I suggest adding a comment to the `DisabledAlgorithms` test to note that
they are different. The one difference I see is that `DisabledAlgorithms` is
also testing that the disabled suites can still be used if the security
properties are empty. Other than that, they look somewhat similar but using
different templates for setting things up.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14316#discussion_r1231245021
