On Thu, 28 Nov 2024 15:30:00 GMT, Alan Bateman <[email protected]> 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
