On Tue, 11 Oct 2022 11:31:37 GMT, Darragh Clarke <[email protected]> wrote:
>> Changed the way the `:authority` pseudo header is set to only include host >> and, if available, port. >> I added a test to cover this change that consists of a HttpClient that makes >> a request which contains userInfo, the test passes if the request is carried >> out with the userInfo not being added to the `:authority` header. >> >> >> ### Tests >> I ran Tier 1 - Tier 3 tests, as well as paying special attention to the http >> client tests to make sure they consistently passed > > Darragh Clarke has updated the pull request incrementally with one additional > commit since the last revision: > > fixed test test/jdk/java/net/httpclient/http2/UserInfoTest.java line 83: > 81: int port = server.getAddress().getPort(); > 82: > 83: server.start(); junit (and even testng) have before/after constructs (for example junit jupiter has `@BeforeAll` and `@AfterAll` https://junit.org/junit5/docs/current/user-guide/#writing-tests-annotations) which get invoked by the test framework. I think moving the server creation and start to `@BeforeAll` and the server stop to `@AfterAll` would be better and will keep the test code in this method simpler. Additionally, you won't need the try/finally block here. In its current form, the server won't get stopped if there are any exceptions thrown in this method before the control reaches `client.send` call. ------------- PR: https://git.openjdk.org/jdk/pull/10592
