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

[Volkan Yazıcı]

On Tue, 26 Nov 2024 09:52:17 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> > @AlanBateman, existing tests are stressing both ctor **and** `connect()`.
> 
> Okay, just a bit surprising to lump them into one test. One thing to consider is one test for the Socket.connect exception scenarios that is focused on checking the connect method throws as expected and the expected Socket state after connect fails. Lots of scenarios to test. A separate test could add to the tests for the Socket constructors, it would be a separate PR if you want.

@AlanBateman, 1e3a52c97f3301b1459b03574c25787a282217f6 splits the introduced tests into individual files:

1. `CloseOnNioConnectFailureTest` – tests 6 scenarios you cited using sockets obtained from `SocketChannel.open(address, port).socket()`
2. `CloseOnOioConnectFailureTest` – tests 6 scenarios you cited using sockets obtained from `Socket(address)` constructors
3. `CloseOnConnectFailureMockTest` – using a `SocketImpl` mock, verifies that the socket is closed on `connect()` failures
4. `CloseOnCtorFailureMockTest` – using a `SocketImpl` mock, verifies that the socket is closed on constructor failures

As you noted, the last one (i.e., `CloseOnCtorFailureMockTest`) is not _directly_ related with this ticket. Though I would appreciate it if you can reconsider keeping it, because:

* a `SocketImpl` mock along with test scenarios implemented for `CloseOnConnectFailureMockTest` are reused to do the 80% of the necessary plumbing
* AFAIK, there are no other tests checking the socket is closed on ctor failures

All in all, I am fine with whatever direction you want me to follow – be it keeping `CloseOnCtorFailureMockTest`, or creating a new ticket+PR for it. How shall I proceed?

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

PR Comment: https://git.openjdk.org/jdk/pull/22160#issuecomment-2501942719