On Tue, 4 Feb 2025 11:22:41 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Replace `assert`s with explicit checks throwing `AssertionError`s
>
> test/jdk/java/net/httpclient/HttpRequestBuilderTest.java line 270:
>
>> 268: HttpRequest defaultHeadReq = new
>> NotOverriddenHEADImpl().HEAD().uri(TEST_URI).build();
>> 269: assertEquals("HEAD", defaultHeadReq.method(), "Method");
>> 270: assertEquals(defaultHeadReq.bodyPublisher().isEmpty(), false,
>> "Body publisher absence");
>
> The params here appear inverted. The first param to the method says
> "expected", so I think it should be:
>
> assertEquals(false, defaultHeadReq.bodyPublisher().isEmpty(), "Body publisher
> absence");
Fixed in 9f7b054b0e96244aea374dbf2555f2a03f61cab0.
> test/jdk/java/net/httpclient/HttpRequestBuilderTest.java line 300:
>
>> 298: + ". Unexpected body processor for GET: "
>> 299: + request.bodyPublisher().get());
>> 300: assertEquals(method, expectedMethod, "Method");
>
> Same here - This should be:
>
>
> assertEquals(expectedMethod, method, "Method");
Fixed in 9f7b054b0e96244aea374dbf2555f2a03f61cab0.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23415#discussion_r1941062879
PR Review Comment: https://git.openjdk.org/jdk/pull/23415#discussion_r1941063050
