On Fri, 29 Nov 2024 15:29:06 GMT, Volkan Yazıcı <d...@openjdk.org> wrote:
>> This PR, addressing 8343791, changes `Socket::connect()` methods to close >> the `Socket` in the event that the connection cannot be established, the >> timeout expires before the connection is established, or the socket address >> is unresolved. >> >> `tier3` tests pass against the 9f00f61d3b7fa42a5e23a04f80bb4bb1a2076ef2. > > Volkan Yazıcı has updated the pull request incrementally with two additional > commits since the last revision: > > - Improve Javadoc > - Match `UHE` message in `Socket` and `SocketImpl` > > This discrepancy was causing following tests to fail: > > - javax/xml/jaxp/unittest/common/dtd/DOMTest.java > - javax/xml/jaxp/unittest/common/dtd/SAXTest.java > - javax/xml/jaxp/unittest/common/catalog/SAXTest.java > - javax/xml/jaxp/unittest/common/catalog/DOMTest.java I think this change looks good now. I'm sure the many iterations were frustrating but I think it has all worked out, and the test is significantly simpler and easy to maintain when compared to where it started. test/jdk/java/net/Socket/ConnectFailTest.java line 48: > 46: * @bug 8343791 > 47: * @summary verifies that `connect()` failures throw expected exception > and leave both `Socket` and the underlying > 48: * `SocketImpl` at the same expected state I think you can drop the mention of the underlying SocketImpl from the summary as the test doesn't do that. ------------- Marked as reviewed by alanb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22160#pullrequestreview-2470280369 PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1863759361
