On Tue, 4 Mar 2025 13:07:37 GMT, Fernando Guallini <fguall...@openjdk.org>
wrote:
> The following tests are marked with @ignore (not running):
>
> - sun/security/ssl/SSLSocketImpl/SetClientMode.java: it checks that setting
> the clientMode after the handshake has begun is not permitted, but this was
> failing intermittently due to a race condition, it was possible that
> SetClientMode was called before the client socket was updated with handshake
> isNegotiated = true. The fix is to introduce a latch to sync between client
> and main threads. Included additional refactoring to ensure test stability.
>
> - sun/security/ssl/SSLSocketImpl/NonAutoClose.java: This test should only run
> in TLS <= 1.2, as TLSv1.3 changes the behaviour of close_notify. Included
> additional refactoring to ensure test stability.
>
> Executed both tests 10K times, no test flakiness found
test/jdk/sun/security/ssl/SSLSocketImpl/NonAutoClose.java line 58:
> 56: */
> 57: private final static String pathToStores =
> "../../../../javax/net/ssl/etc";
> 58: private final static String keyStoreFile = "keystore";
This isn't in the changeset, but we're trying to move away from the binary
{key|trust}store files. We have two template classes, SSLContextTemplate and
SSLSocketTemplate, that set up key and trust stores and does the context setup.
Can you take a look and see if one of them can be used here?
test/jdk/sun/security/ssl/SSLSocketImpl/NonAutoClose.java line 70:
> 68: * Turn on SSL debugging?
> 69: */
> 70: private final static boolean DEBUG = false;
I know this isn't technically in the change set but when I see this I change it
to `DEBUG = Boolean.getBoolean("test.debug")` so the flag can be toggled
without rebuilding the code.
test/jdk/sun/security/ssl/SSLSocketImpl/NonAutoClose.java line 77:
> 75: private final static int TLS_CLIENT_VAL = 4;
> 76:
> 77: private void expectValue(int got, int expected, String msg) throws
> IOException {
Can you just use the Asserts class instead?
test/jdk/sun/security/ssl/SSLSocketImpl/SetClientMode.java line 66:
> 64: * Where do we find the keystores?
> 65: */
> 66: private final static String pathToStores =
> "../../../../javax/net/ssl/etc";
Could you use either SSLContextTemplate or SSLSocketTemplate for this test?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r1981466359
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r1981448111
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r1981443329
PR Review Comment: https://git.openjdk.org/jdk/pull/23898#discussion_r1981542780
