RFR: 8249825: Tests sun/security/ssl/SSLSocketImpl/SetClientMode.java and NonAutoClose.java marked with @ignore [v2]
Fernando Guallini
fguallini at openjdk.org
Thu Mar 20 11:59:07 UTC 2025
On Tue, 18 Mar 2025 02:27:31 GMT, Artur Barashev <abarashev at openjdk.org> wrote:
>> Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:
>>
>> SSLContextTemplate and using asserts
>
> test/jdk/sun/security/ssl/SSLSocketImpl/NonAutoClose.java line 65:
>
>> 63: */
>> 64: private final static boolean DEBUG = Boolean.getBoolean("test.debug");
>> 65: private final static int NUM_ITERATIONS = 10;
>
> Why do we do 10 iterations of the same test?
By looking at the original issue, I believe the multiple iterations were added to ensure that the synchronised behaviour is consistent over several cycles of socket opening and closing, to test that there is no race condition when waiting for the close_notify.
> test/jdk/sun/security/ssl/SSLSocketImpl/NonAutoClose.java line 135:
>
>> 133: */
>> 134: System.out.println("Waiting for server ready");
>> 135: if (!SERVER_READY.await(5, TimeUnit.SECONDS)) {
>
> Please see my other comment about `await()`.
Yes, that is fine, updated
> test/jdk/sun/security/ssl/SSLSocketImpl/SetClientMode.java line 100:
>
>> 98: connectedSocket.getSession();
>> 99:
>> 100: if (!HANDSHAKE_COMPLETE.await(5, TimeUnit.SECONDS)) {
>
> The test will be more robust if we do a simple `await()` call here. On some busy systems even 5 seconds may not be enough. JTEG tests have a default timeout of 2 minutes, so it won't wait forever in any case.
Yes, that is fine, updated
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r2005462612
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r2005463263
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r2005464583
More information about the security-dev
mailing list