On Mon, 3 Feb 2025 19:33:23 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
>> Adds `HttpRequest.Builder::copy` tests to `HttpRequestBuilderTest`. > > 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23415#discussion_r1940628336
