On Mon, 6 May 2024 05:36:10 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I get a review of this test-only fix which addresses the intermittent >> failrues in `com/sun/net/httpserver/HttpsParametersClientAuthTest.java` on >> Windows? >> >> As noted in https://bugs.openjdk.org/browse/JDK-8331334, the test relies on >> the TLS communication (of a HTTP request) to fail and then asserts on the >> exception types. On Windows, sometimes the exception type differs, although >> the underlying failure is the expected failure. >> >> The commit in this PR adds an additional condition to allow for >> `IOException` to be thrown when on Windows. The exception type and message >> check is additionally only added for Windows and the message check is very >> specific intentionally, to make sure we don't allow any `IOException` to >> result in the test passing. The usage of the `WSAECONNABORTED_MSG` has a >> precedent in `test/jdk/java/net/httpclient/HandshakeFailureTest.java`, so I >> think that should be OK in this test too. >> >> With these changes the test hasn't failed in a test repeat of 100. Without >> the changes, the test fails around 15 times in a test repeat of 100. >> Additionally tier1, tier2 and tier3 tests have been run with this change and >> those tests continue to pass. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > don't check for exception message (which can be localized) test/jdk/com/sun/net/httpserver/HttpsParametersClientAuthTest.java line 226: > 224: } > 225: // verify it failed due to right reason > 226: Throwable cause = ioe.getCause(); shouldn't we start with `cause = ioe` like before? Or was this done to make sure we only look at the nested exception on windows? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19091#discussion_r1592327204
