On Sat, 21 Mar 2026 01:35:43 GMT, Bradford Wetmore <[email protected]> wrote:
>> test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketSSLEngineCloseInbound.java
>> line 95:
>>
>>> 93: * Enables logging of the SSL/TLS operations.
>>> 94: */
>>> 95: private static final boolean logging = true;
>>
>> Since this logging is always set to true I personally don't think this
>> should be even included. What do you think about removing it and always
>> logging?
>
> I have no objections to the additional configuration options. (i.e. leaving
> as is.)
Keep the logging option.
>> test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketSSLEngineCloseInbound.java
>> line 106:
>>
>>> 104: * after gaining some familiarity with this application.
>>> 105: */
>>> 106: private static final boolean debug = false;
>>
>> How abut making the ssl debug option as a part of `@run`? It is always false
>> as well which means that it does nothing atm.
>>
>> If you believe it's needed, could you please change it so it takes a boolean
>> system property `test.debug`? Like this:
>> ```java
>> Boolean.getBoolean("test.debug");
>
> There is no need to output the ssl debugging information by default, it just
> adds lots of extra output that is usually never looked at. If there is a
> problem with this test, developers can quickly toggle the switch to get the
> needed debug info.
>
> We just turned the default JSSE debug output off in many unneeded test cases.
> ([JDK-8368493](https://bugs.openjdk.org/browse/JDK-8368493)) No need to
> turn it back on.
>
> During the review of JDK-8368493, there was a suggestion to update all of the
> JSSE test cases to use `test.debug`. We could consider that, but let's do
> that as a unified chunk of work, rather than 1-offs here.
>
> Thanks.
Agreed, so we will have a unified set of changes across the JSSE test cases.
>> test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketSSLEngineCloseInbound.java
>> line 296:
>>
>>> 294:
>>> 295: // Server signals client it has finished
>>> writing
>>> 296: serverReadyLatch.countDown();
>>
>> minor: what if exceptions are thrown, would the test just time out? Wouldn't
>> it make sense to release the latch on the exceptions too?
>
> Or in the finally block?
Done. Release the latch in the finally block.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30340#discussion_r2978900935
PR Review Comment: https://git.openjdk.org/jdk/pull/30340#discussion_r2978901184
PR Review Comment: https://git.openjdk.org/jdk/pull/30340#discussion_r2978901944
