On Tue, 12 Nov 2024 15:03:45 GMT, Artur Barashev <abaras...@openjdk.org> wrote:
>> test/jdk/sun/security/ssl/CipherSuite/TLSCipherSuiteWildCardMatchingIllegalArgument.java >> line 47: >> >>> 45: * class. Thus, we need a separate test class each time we need to >>> modify >>> 46: * "jdk.tls.disabledAlgorithms" config value for testing. >>> 47: */ >> >> Still nitpick-level and it might violate coding rules I'm not aware of: >> Declaring `wildCardMatch` package visible should allow you to test that >> methods's behaviour by simply calling it with fitting parameters in the >> other test class. Pass the cache-Map as parameter instead of accessing the >> "global variable" in the method and you can additionally test the caching >> mechanism. > > Yes, I thought about that as well. But I think modifying method's visibility > just to accommodate tests goes against current JDK coding conventions. > @seanjmullan May correct me on that. Right, changing the method's visibility just to accommodate testing is not a good idea. It can be done in other ways, ex: using reflection but I don't see a compelling enough reason to test the internal methods here. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21841#discussion_r1838721621
