RFR: 8378344: Refactor test/jdk/java/net/httpclient/*.java TestNG tests to JUnit

[Volkan Yazici]

On Fri, 20 Feb 2026 19:23:00 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> TestNG tests directly located in test/jdk/java/net/httpclient/ are refactored to use JUnit.
> 
> This is a mostly mechanical change - though I fixed some compiker warnings along teh way.
> 
> Test only.

I've tried to skim through all the changes. Thanks so much for keeping the `diff` as small as possible, it helped.

test/jdk/java/net/httpclient/AbstractNoBody.java line 58:

> 56: // to this.getClass() in methods that are called from
> 57: // @BeforeAll and @AfterAll
> 58: @TestInstance(TestInstance.Lifecycle.PER_CLASS)

AFAICS, judging from `AbstractNoBody` and all its subclasses, `AbstractNoBody::printStamp` is the only place needing `getClass()`. You can alternatively consider simply removing that usage — I doubt if it is adding much value.

test/jdk/java/net/httpclient/BufferingSubscriberErrorCompleteTest.java line 124:

> 122:         assertEquals(0, exposingSubscriber.onCompleteInvocations);
> 123:         assertEquals(t, exposingSubscriber.throwable);
> 124:         assertEquals(                     "a message from me to me", exposingSubscriber.throwable.getMessage());

Suggestion:

        assertEquals("a message from me to me", exposingSubscriber.throwable.getMessage());

test/jdk/java/net/httpclient/CookieHeaderTest.java line 177:

> 175:             assertEquals(                    cookies.stream()
> 176:                             .filter(s -> !s.startsWith("LOC"))
> 177:                             .collect(Collectors.toList()), response.headers().allValues("X-Request-Cookie"));

There is a syntax issue. Putting that aside, it might be better to create two `{expected,actual}Cookies` variables to decrease the cognitive load here.

test/jdk/java/net/httpclient/DependentActionsTest.java line 253:

> 251:         HttpClient shared = sharedClient;
> 252:         if (shared != null) return shared;
> 253:         synchronized (zis) {

I presume you opted for `zis` 😅 approach to minimize the `diff`, which makes sense. I don't have an objection. Alternatively, you can consider making `sharedClient` a `LazyConstant`, or getting it initialized in `@BeforeAll`.

Note that there are more `zis` in this PR where this remark applies.

test/jdk/java/net/httpclient/FlowAdapterSubscriberTest.java line 491:

> 489:                     .thenApply(FlowAdapterSubscriberTest::assert200ResponseCode)
> 490:                     .thenApply(HttpResponse::body)
> 491:                     .thenAccept(body -> assertEquals("We're sucking diesel now!", body))

I'm having a very educational code review experience! :star_struck:

> "Now we're sucking diesel" is a common Irish slang phrase meaning that a project, task, or situation has finally gained momentum, is running smoothly, or is progressing successfully.

test/jdk/java/net/httpclient/HttpVersionsTest.java line 166:

> 164:             assertEquals(HTTP_1_1, response.version());
> 165:             assertEquals("", response.body());
> 166:             assertEquals(                         "HTTP/1.1 request received by HTTP/2 server", response.headers().firstValue("X-Magic").get());

Suggestion:

            assertEquals("HTTP/1.1 request received by HTTP/2 server", response.headers().firstValue("X-Magic").get());

test/jdk/java/net/httpclient/HttpVersionsTest.java line 196:

> 194:             assertEquals(HTTP_1_1, response.version());
> 195:             assertEquals("", response.body());
> 196:             assertEquals(                         "HTTP/1.1 request received by HTTP/2 server", response.headers().firstValue("X-Magic").get());

Suggestion:

            assertEquals("HTTP/1.1 request received by HTTP/2 server", response.headers().firstValue("X-Magic").get());

test/jdk/java/net/httpclient/UserCookieTest.java line 186:

> 184:             assertEquals(                    expectedCookies.stream()
> 185:                             .filter(s -> !s.startsWith("LOC"))
> 186:                             .toList(), response.headers().allValues("X-Request-Cookie"));

Whitespace needs to be removed. You can also consider creating `{expect,actual}Cookies` variables.

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

PR Review: https://git.openjdk.org/jdk/pull/29850#pullrequestreview-3845866292
PR Review Comment: https://git.openjdk.org/jdk/pull/29850#discussion_r2845364005
PR Review Comment: https://git.openjdk.org/jdk/pull/29850#discussion_r2845198633
PR Review Comment: https://git.openjdk.org/jdk/pull/29850#discussion_r2845219199
PR Review Comment: https://git.openjdk.org/jdk/pull/29850#discussion_r2845234411
PR Review Comment: https://git.openjdk.org/jdk/pull/29850#discussion_r2845287828
PR Review Comment: https://git.openjdk.org/jdk/pull/29850#discussion_r2845320650
PR Review Comment: https://git.openjdk.org/jdk/pull/29850#discussion_r2845321932
PR Review Comment: https://git.openjdk.org/jdk/pull/29850#discussion_r2845932996