RFR: 8349135: Add tests for HttpRequest.Builder.copy() [v3]

Volkan Yazici vyazici at openjdk.org
Tue Feb 4 11:07:11 UTC 2025


On Tue, 4 Feb 2025 07:24:20 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Volkan Yazici has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'upstream/master' into HttpRequest-Builder-copy-Test
>>  - Update copyright year
>>  - Verify `copy()` in `HttpRequestBuilderTest`
>
> test/jdk/java/net/httpclient/HttpRequestBuilderTest.java line 407:
> 
>> 405: 
>> 406:         // Verify copied _references_
>> 407:         assert request.uri().equals(copiedRequest.uri());
> 
> Hello Volkan, within the JDK build, we launch these jtreg tests with assertions enabled by default. However, from what I see in the build files, that's configurable. So it may not always be the case that assertions are enabled for these tests.
> 
> Furthermore, I don't remember seeing `assert` being used within the test code to verify the test expectations. So I think it would be better to do a conditional check here and explicitly throw a `AssertionError` if that check fails, like we do in some other places of this test.

Fixed in 5eddbf7d4477cf997111b4dae8b3cb112216fadc.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23415#discussion_r1940959574


More information about the net-dev mailing list