On Thu, 28 Nov 2024 15:44:23 GMT, Volkan Yazıcı <d...@openjdk.org> wrote:
>> 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?
I checked the socket adaptor and there is needed a bug here. SocketChannel
throws AlreadyConnectedException and the socket adaptor should map that to
SocketException("already connected"). It doesn't do that so we should fix it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1862411995
