On Fri, 19 Sep 2025 10:44:36 GMT, Mahendra Chhipa <[email protected]> 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 test/jdk/java/net/httpclient/http2/ConnectionFlowControlTest.java line 75: > 73: import org.junit.jupiter.params.provider.MethodSource; > 74: > 75: import static org.junit.jupiter.api.Assertions.*; This test uses `assertEquals` and `assertNotEquals`. Unless I'm mistaken, the parameters order are reversed in JUnit compared to TestNG. That is, one has (expected, actual), the other has (actual, expected). Unless there was already a mistake in the original code, you should also reverse the parameters order at the call sites of `assertEquals`/`assertNotEquals`. Otherwise, the text message of any `AssertionError` thrown by these will be confusing. test/jdk/java/net/httpclient/http2/ConnectionFlowControlTest.java line 265: > 263: > 264: http2TestServerLocal.start(); > 265: https2TestServerLocal.start(); I believe you should revert these changes or make sure they result in exactly the same calls. For instance - `http2TestServerLocal.start();` is not exactly the same as `this.http2TestServer.start();` - the original code was calling start() on the wrapper, and IMO it's better to continue doing that. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2368766736 PR Review Comment: https://git.openjdk.org/jdk/pull/27388#discussion_r2368799614
