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 Mon, 25 Nov 2024 10:01:40 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> You shouldn't need a mock SocketImpl. I assume you have this because the test is testing the Socket constructors mostly.

@AlanBateman, existing tests are stressing both ctor **and** `connect()`.

AFAICT, a `SocketImpl` mock is required to verify if the `Socket` ctor closes the `SocketImpl` correctly. I don't know any other way to perform this check.

I also used the `SocketImpl` mock to verify the behavior of `connect()` on

1. `UHE`-causing input
2. Various `SocketImpl`-thrown exceptions

The mock enabled the tests to only focus on what needs to be verified and exclude every other variable.

> I see Mark is reviewing the test so I didn't put much time into looking at the test. The main thing is for this test is that it exercises Socket.connect and uses Socket::isClosed to test if the Socket is closed or not after the connect fails. I think we need to test at least the following:
> 
> * Socket unbound and unconnected, Socket connect fails, Socket should be closed
> * Socket bound and unconnected, Socket connect fails, Socket should be closed
> * Socket connected, Socket connect fails, Socket should not be closed
> * Socket unbound and unconnected, Socket connect to unresolved address, Socket should be closed
> * Socket bound and unconnected, Socket connect to unresolved address, Socket should be closed
> * Socket connected, Socket connect connect to unresolved address Socket should not be closed
> 
> You shouldn't need a mock SocketImpl. I assume you have this because the test is testing the Socket constructors mostly.
> 
> For the socket adapter then you could have the tests be parametrised so they are called with an newly created Socket (unbound/unconnected), that way you can test new Socket() and SocketChannel.open().socket().

I've pushed changes implementing explicit tests for each cited 6 scenarios and using two `Socket` factories:

1. The `Socket` constructor
2. `Socket.open().socket()`

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

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