On Fri, 19 Sep 2025 10:44:36 GMT, Mahendra Chhipa <mchh...@openjdk.org> wrote:
> Refactoring following httpclient/http2 testng test to JUnit :
> test/jdk/java/net/httpclient/http2/BadHeadersTest.java
> test/jdk/java/net/httpclient/http2/BadPushPromiseTest.java
> test/jdk/java/net/httpclient/http2/BasicTest.java
> test/jdk/java/net/httpclient/http2/ConnectionFlowControlTest.java
> test/jdk/java/net/httpclient/http2/ContinuationFrameTest.java
@mahendrachhipa, would you mind confirming that `tier1,2` pass, please?
test/jdk/java/net/httpclient/http2/BadHeadersTest.java line 88:
> 86: );
> 87:
> 88: SSLContext sslContext;
These can be `private` for better encapsulation.
test/jdk/java/net/httpclient/http2/BadHeadersTest.java line 145:
> 143: Arguments.of(http2URI, true, byteAtATime),
> 144: Arguments.of(https2URI, true, byteAtATime)
> 145: );
AFAICT, you don't need to return a `Stream<Arguments>`, the old `Object[][]`
should suffice. Maybe that is better to revert this to old style to keep the
diff minimal – granted `Object[][]` indeed works with JUnit.
test/jdk/java/net/httpclient/http2/BadHeadersTest.java line 247:
> 245:
> 246: @BeforeAll
> 247: public static void setup() throws Exception {
`setup()` and `teardown()` can be package-private.
Note that this comment applies to all `@AfterAll`- and `@BeforeAll`-annotated
methods also present in other touched files.
test/jdk/java/net/httpclient/http2/BasicTest.java line 129:
> 127:
> 128: @Test
> 129: public void test() throws Exception {
`@[Parameterized]Test`-annotated methods can be package-private.
test/jdk/java/net/httpclient/http2/ConnectionFlowControlTest.java line 83:
> 81: static HttpTestServer https2TestServer; // HTTP/2 ( h2 )
> 82: static String http2URI;
> 83: static String https2URI;
These can be `private`.
test/jdk/java/net/httpclient/http2/ConnectionFlowControlTest.java line 90:
> 88: Arguments.of(http2URI),
> 89: Arguments.of(https2URI)
> 90: );
You can use `Object[][]`.
test/jdk/java/net/httpclient/http2/ContinuationFrameTest.java line 82:
> 80: static String noBodyhttp2URI;
> 81: static String noBodyhttps2URI;
> 82: final static ReferenceTracker TRACKER = ReferenceTracker.INSTANCE;
These can be `private`.
test/jdk/java/net/httpclient/http2/ContinuationFrameTest.java line 137:
> 135: };
> 136:
> 137: static Stream<Arguments> variants() {
This can be `Object[][]`.
-------------
Changes requested by vyazici (Committer).
PR Review: https://git.openjdk.org/jdk/pull/27388#pullrequestreview-3251038574
PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2367030229
PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2367041647
PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2367032448
PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2367043354
PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2367045449
PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2367045017
PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2367060462
PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2367061035
