On Fri, 12 Nov 2021 10:15:29 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with four additional
>> commits since the last revision:
>>
>> - Implement Daniel's review suggestion - use a different URI for HEAD()
>> request in HttpRequestNewBuilderTest
>> - Remove unused imports and fields from HeadTest
>> - - Update the HeadTest to use the convenience request builder methods
>> GET() and HEAD()
>> - Also add @bug tag to updated tests
>> - fix comment about special casing certain HTTP methods
>
> src/java.net.http/share/classes/java/net/http/HttpRequest.java line 382:
>
>> 380: case "GET" -> builder.GET();
>> 381: case "DELETE" -> builder.DELETE();
>> 382: case "HEAD" -> builder.HEAD();
>
> The comment above (line 376) mentions:
>
> // otherwise, the body is absent, special case for GET/DELETE,
> // or else use empty body
>
> maybe HEAD should be added there as well...
You're right. I missed that. I've updated the PR to fix this.
> test/jdk/java/net/httpclient/HttpRequestNewBuilderTest.java line 124:
>
>> 122: // dedicated method
>> 123: {
>> HttpRequest.newBuilder(URI.create("https://method-1/";)).GET().build() },
>> 124: {
>> HttpRequest.newBuilder(URI.create("https://method-1/";)).HEAD().build() },
>
> I'd suggest to change the URI path to `/method-0/` and move the HEAD case to
> just after line 122 (before GET)
Done in the updated version of the PR
-------------
PR: https://git.openjdk.java.net/jdk/pull/6348
