On Thu, 5 Nov 2020 17:08:10 GMT, Patrick Concannon <pconcan...@openjdk.org>
wrote:
>> Hi,
>>
>> Could someone please review my fix for JDK-8253005: 'Add `@throws
>> IOException` in javadoc for `HttpEchange.sendResponseHeaders`' ?
>>
>> The method `HttpEchange.sendResponseHeaders` throws an `IOException` but is
>> unspecified in its javadoc. This fix adds an `@throws IOException` to its
>> specification and a description of the conditions under which the exception
>> is thrown.
>>
>> Kind regards,
>> Patrick
>
> 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
Changes requested by chegar (Reviewer).
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.
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?
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?
test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 99:
> 97: // unexpected exception thrown, return error to client
> 98: t.printStackTrace();
> 99: os.write(("Unexpected error: " + t).getBytes());
Hmm... I'd be tempted to drop this catch block altogether, it's not possible to
know exactly what to do there? given that we don't know what the failure is.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1014
