On Fri, 29 Nov 2024 11:38:05 GMT, Alan Bateman <[email protected]> wrote:
>> Volkan Yazıcı has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Use more fine-grained suppression
>> - Further simplify tests
>> - Extend `translateToSocketException()` with `AlreadyConnectedException`
>
> test/jdk/java/net/Socket/ConnectFailTest.java line 189:
>
>> 187:
>> 188: @SuppressWarnings("ResultOfMethodCallIgnored")
>> 189: private static void continuouslyAcceptConnections(ServerSocket
>> serverSocket) {
>
> I realize the test has gone through several rounds of simplification. I think
> it can be simplified further as there is no need to use an ExecutorService or
> have another thread managing a listen socket and accepting connections. The
> reason is that all the supported operating systems queue accepted
> connections. This means it can be all done synchronously, e.g.
> testConnectedSocket and testConnectedSocketWithUnresolvedAddress can setup
> the loopback connection with code like this:
>
> try (socket) {
> withListener(listener -> {
> socket.connect(listener.getLocalSocketAddress());
> try (var peer = listener.accept()) {
> :
> }
> });
> }
That swiftly yielded 10 insertions and 56 deletions! 💥 (Implemented in
33fad083d9b3b4a0ed753e2a9b179e9bbf87387c.)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1863538348