On Tue, 3 Dec 2024 10:50:37 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this test-only change which should improve the > debuggability of the `test/jdk/java/net/Socket/CloseAvailable.java` test? > > The test has been intermittently failing in our CI as noted in > https://bugs.openjdk.org/browse/JDK-8269526. With these added logs in the > test, I think it should help in the failure investigations in any future > failures in this test. > > Repeat testing of this test in our CI, with this change, has passed. A tier2 > testing is in progress. The logic of the tests didn't change by this refactoring - the logging output printed to stdout while running and also the detail messages added to `assert`-statements will improve the debuggability. test/jdk/java/net/Socket/CloseAvailable.java line 28: > 26: * @bug 4091859 8189366 > 27: * @library /test/lib > 28: * @summary Test Socket.getInputStream().available() Bit-rot in documentation, good catch. test/jdk/java/net/Socket/CloseAvailable.java line 87: > 85: final int av = is.available(); > 86: // available() was expected to fail but didn't > 87: throw new AssertionError("Socket.getInputStream().available() > was expected to fail on " Throwing an assertion error directly here is much better then the former fall-through approach. test/jdk/java/net/Socket/CloseAvailable.java line 102: > 100: System.out.println("testEOF, readUntilEOF: " + readUntilEOF); > 101: final InetAddress addr = InetAddress.getLoopbackAddress(); > 102: try (final ServerSocket ss = new ServerSocket()) { Seems like now the server socket is automatically closed after usage. Good! ------------- Marked as reviewed by cstein (Committer). PR Review: https://git.openjdk.org/jdk/pull/22511#pullrequestreview-2475344449 PR Review Comment: https://git.openjdk.org/jdk/pull/22511#discussion_r1867525270 PR Review Comment: https://git.openjdk.org/jdk/pull/22511#discussion_r1867529207 PR Review Comment: https://git.openjdk.org/jdk/pull/22511#discussion_r1867531600
