On Mon, 15 Dec 2025 13:10:04 GMT, Jaikiran Pai <[email protected]> wrote:
>> @jaikiran Thank you for the detailed review! I've addressed all your >> feedback: >> >> 1. **Http2Connection.java**: Removed the `else` block so `close(cause)` → >> `doTerminate()` handles processed streams. This ensures connection closure >> cause and stream termination cause are consistent. >> >> 2. **GoAwayWithErrorTest.java**: >> - Using `URIBuilder` with server's actual address instead of "localhost" >> - Removed timeout from `goAwaySentLatch.await()` >> - Removed entire `allOf().orTimeout().join()` block >> - Replaced success counting with direct `assertEquals(200, >> response.statusCode())` > > Thank you @ehs208 for these updates. The changes look good to me. I'll run > the latest changes in our CI to make sure this continues to pass. > > On a related note, have you run tier2 locally with these changes? The GitHub > actions job only run tier1 and networking tests aren't covered in that. It > would be good to run tier2 on your local setup too - that way any future > contributions in this area can be tested locally to verify that changes don't > break unexpected existing tests. @jaikiran I ran jdk_net tests locally. Results: 1047 passed, 1 failed. The failed test is `HTTPSetAuthenticatorTest` (HTTP/1.1 Authenticator test) with `SocketException: Connection reset`, which appears to be a flaky test unrelated to this PR's HTTP/2 GOAWAY changes. All HTTP/2 related tests passed successfully. ------------- PR Comment: https://git.openjdk.org/jdk/pull/28632#issuecomment-3659826810
