8381842: Refactor remaining TestNG tests under java/net/ to use JUnit by dfuch · Pull Request #30681 · openjdk/jdk

dfuch · 2026-04-10T17:41:34Z

Hi, may I get a review for this change that converts the remaining TestNG tests under test/jdk/java/net to JUnit.

An exception is java/net/NetworkInterface/NetworkInterfaceStreamTest.java which depends on library classes depending on TestNG. Converting that test is not done here but is tracked by JDK-8381848

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8381842: Refactor remaining TestNG tests under java/net/ to use JUnit (Sub-task - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30681/head:pull/30681
$ git checkout pull/30681

Update a local copy of the PR:
$ git checkout pull/30681
$ git pull https://git.openjdk.org/jdk.git pull/30681/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 30681

View PR using the GUI difftool:
$ git pr show -t 30681

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30681.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-04-10T17:43:36Z

👋 Welcome back dfuchs! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-04-10T17:44:00Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2026-04-10T17:45:00Z

@dfuch The following label will be automatically applied to this pull request:

net

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2026-04-10T17:49:05Z

Webrevs

00: Full (5ff87a55)

Marcono1234 · 2026-04-10T20:29:55Z

test/jdk/java/net/DatagramPacket/Setters.java

@@ -53,8 +54,7 @@ public void testSetAddress() throws Exception {
        assertTrue(pkt.getAddress() == null);


assertNull?

Marcono1234 · 2026-04-10T20:33:25Z

test/jdk/java/net/InetAddress/HostsFileOrderingTest.java

                .map(InetAddress::getHostAddress).toArray(String[]::new);
        String [] expectedAddresses = getExpectedAddressesArray();

        if (Arrays.deepEquals(resolvedAddresses, expectedAddresses)) {


Could use assertArrayEquals? (unless you want to keep the custom error messages)

Marcono1234 · 2026-04-10T20:38:33Z

test/jdk/java/net/InetSocketAddress/ToString.java

    }

    @Test
    // InetSocketAddress.toString() throws NPE with unresolved address


This comment is wrong / misleading; it was the behavior before https://bugs.openjdk.org/browse/JDK-4464064 was fixed

Maybe would also be good to rename the method then from NPETest to unresolvedTest or similar?

Marcono1234 · 2026-04-10T20:38:51Z

test/jdk/java/net/InetSocketAddress/ToString.java

    // InetSocketAddress.toString() throws NPE with unresolved address
-    public static void NPETest() {
+    public void NPETest() {
        System.out.println(new InetSocketAddress("unresolved", 12345));


Replace with assertDoesNotThrow / assertNotNull?

Marcono1234 · 2026-04-10T20:41:12Z

test/jdk/java/net/InetSocketAddress/ToString.java


-    @DataProvider(name = "hostPortArgs")
-    public Object[][] createArgs1() {
+    public static Object[][] createArgs1() {


Would it make sense to apply the provider name as method name (i.e. hostPortArgs) to make it more expressive?

(same also for the other argument methods here)

Marcono1234 · 2026-04-10T21:08:45Z

test/jdk/java/net/Socks/SocksIPv6Test.java

+    static private HttpServer server;
+    static private SocksServer socks;


Should be private static instead of static private to be conventional?

Marcono1234 · 2026-04-10T21:09:31Z

test/jdk/java/net/Socks/SocksIPv6Test.java

-    private String response = "Hello.";
+    static private HttpServer server;
+    static private SocksServer socks;
+    private static String response = "Hello.";


Can be final?

Marcono1234 · 2026-04-10T21:14:56Z

test/jdk/java/net/Socks/SocksSocketImplTest.java

        try (ServerSocket ss = new ServerSocket(port, backlog, InetAddress.getLoopbackAddress());
             Socket s1 = new Socket(ss.getInetAddress(), ss.getLocalPort());
             Socket s2 = ss.accept()) {
-            Assert.fail("IOException was expected to be thrown, but wasn't");
+                fail("IOException was expected to be thrown, but wasn't");
        } catch (IOException ioe) {
            // expected
            // now verify the IOE was thrown for the correct expected reason
            if (!(ioe.getCause() instanceof IllegalArgumentException)) {


Use assertThrows? E.g.:

var e = assertThrows(IOException.class, ...); assertInstanceOf(IllegalArgumentException.class, e.getCause());

That would also make it clearer which of the calls is expected to throw the IOException.

Marcono1234 · 2026-04-10T21:18:36Z

test/jdk/java/net/UnixDomainSocketAddress/LengthTest.java

        if (namelen == -1)
-            return new Object[][] {new String[]{""}};
+            return List.of("");


Remove this namelen == -1 check because it is always false?

Marcono1234 · 2026-04-10T21:19:37Z

test/jdk/java/net/UnixDomainSocketAddress/LengthTest.java

+                new String(new char[100]).replaceAll("\0", "x"),
+                new String(new char[namelen]).replaceAll("\0", "x"),
+                new String(new char[namelen-1]).replaceAll("\0", "x")


Can simplify these?

Suggested change

new String(new char[100]).replaceAll("\0", "x"),

new String(new char[namelen]).replaceAll("\0", "x"),

new String(new char[namelen-1]).replaceAll("\0", "x")

"x".repeat(100),

"x".repeat(namelen),

"x".repeat(namelen - 1)

8381842: Refactor remaining TestNG tests under java/net/ to use JUnit

5ff87a5

openjdk bot added the net net-dev@openjdk.org label Apr 10, 2026

openjdk bot added the rfr Pull request is ready for review label Apr 10, 2026

Marcono1234 reviewed Apr 10, 2026

View reviewed changes

		@@ -53,8 +54,7 @@ public void testSetAddress() throws Exception {
		assertTrue(pkt.getAddress() == null);

		static private HttpServer server;
		static private SocksServer socks;

Conversation

dfuch commented Apr 10, 2026 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Apr 10, 2026

Uh oh!

openjdk bot commented Apr 10, 2026

Uh oh!

openjdk bot commented Apr 10, 2026

Uh oh!

mlbridge bot commented Apr 10, 2026

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

dfuch commented Apr 10, 2026 •

edited by openjdk bot

Loading