On Thu, 5 Nov 2020 17:30:32 GMT, Chris Hegarty <che...@openjdk.org> wrote:
>> Patrick Concannon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8252304: Added read to TestHandler to ensure requestBody consumed before >> closing exchange > > test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 75: > >> 73: .GET() >> 74: .build(); >> 75: HttpResponse<String> response = client.send(request, >> HttpResponse.BodyHandlers.ofString()); > > How about we import java.net.http.HttpResponse.BodyHandlers? Added import for java.net.http.HttpResponse.BodyHandlers. See https://github.com/openjdk/jdk/pull/1014/commits/8ff6d9b4fcbc0f57f136ff0e7df0d371664badb2 > test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 69: > >> 67: server.getAddress().getPort(), >> 68: path, null, null); >> 69: > > This maybe fine, but the pattern we've used elsewhere ( and have proven to be > reliable ) is: InetAddress.getLoopbackAddress().getHostName() and > server.getAddress().getPort(), but now I think that maybe these could be an > issue for IPv6-only hosts? `server.getAddress().getHostString()` replaced with `InetAddress.getLoopbackAddress().getHostName()` as requested. See https://github.com/openjdk/jdk/pull/1014/commits/8ff6d9b4fcbc0f57f136ff0e7df0d371664badb2 > test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 63: > >> 61: ExecutorService executor = Executors.newCachedThreadPool(); >> 62: server.setExecutor (executor); >> 63: server.start(); > > Being a little pedantic, then the setup and teardown of the server could be > moved into an `@BeforeTest` and `@AfterTest`, which would leave the client > part of the test uncluttered. Restructured to use @BeforeTest and @AfterTest. See https://github.com/openjdk/jdk/pull/1014/commits/8ff6d9b4fcbc0f57f136ff0e7df0d371664badb2 ------------- PR: https://git.openjdk.java.net/jdk/pull/1014
