RFR: 8343791: Socket.connect API should document whether the socket will be closed when hostname resolution fails or another error occurs [v22]

[Volkan Yazıcı]

On Thu, 28 Nov 2024 15:30:00 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> It would make them similar to the other test methods but would be a bit strange since the test method would be called only once with a single parameter set. 
>> I mean, the other test methods get two sockets, but these will only ever take one.
>
> It should be called twice with a new created Socket.  Socket.connect is specified to throw IOException if already connected, we shouldn't be throwing (or testing for) AlreadyConnectedException here. It may be that Net.translateToSocketException is missing that mapping (we should check that).

I need to verify the thrown exception in the test:

* If the socket passed created using `Socket::new`, a `SocketException("already connected")` is thrown
* If the socket passed created using `SocketChannel.open().socket()`, an `AlreadyConnectedException` is thrown

AFAICT, the verifier needs to be passed along with the socket, and this implies either separate tests, which is what the current code does, or a separate test method source as in


static List<Arguments> connectedSocketFactoriesAndReconnectFailureVerifiers() {
    return List.of(
            Arguments.of(
                    (Function<SocketAddress, Socket>) ConnectFailTest::createConnectedSocket,
                    (Consumer<Executable>) executable -> {
                        SocketException exception = assertThrows(SocketException.class, executable);
                        assertEquals("already connected", exception.getMessage());
                    }),
            Arguments.of(
                    (Function<SocketAddress, Socket>) ConnectFailTest::createConnectedNioSocket,
                    (Consumer<Executable>) executable -> assertThrows(AlreadyConnectedException.class, executable))
    );
}


Am I mistaken?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1862410159