On Fri, 29 Nov 2024 11:38:05 GMT, Alan Bateman <al...@openjdk.org> 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