On Mon, 30 Mar 2026 18:08:24 GMT, Daniel Fuchs <[email protected]> wrote:
> May I get a review for this change that converts `java/net/DatagramSocket`
> TestNG tests to use JUnit.
>
> Some of these tests now take advantage from the fact that Closeable arguments
> passed to a ParameterizedTest are automatically closed by JUnit.
test/jdk/java/net/DatagramSocket/ConnectPortZero.java line 77:
> 75: @ParameterizedTest
> 76: @MethodSource("testCases")
> 77: public void testConnect(DatagramSocket ds, InetAddress addr) {
What would you think about using `try (ds) { .. }` so that it's clear that the
DatagramSocket will be closed.
test/jdk/java/net/DatagramSocket/DatagramTimeout.java line 57:
> 55: public static List<DatagramSocket> sockets() throws IOException {
> 56: // Closeable arguments passed to a ParameterizedTest are
> automatically
> 57: // closed by JUnit
I personally don't like to rely on that feature, only because it's a bit too
magic.
test/jdk/java/net/DatagramSocket/OldDatagramSocketImplTest.java line 57:
> 55: } catch (SocketException ex) {
> 56: assertEquals("connect not implemented", ex.getMessage());
> 57: System.out.println("PASSED: default implementation of connect
> has thrown as expected");
Is this trace message needed? I'm reminded that our the JUnit output goes to
System.err whereas TestNG output goes to System.out, so any trace messages to
System.out will no longer be inlined.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30502#discussion_r3013797130
PR Review Comment: https://git.openjdk.org/jdk/pull/30502#discussion_r3013803020
PR Review Comment: https://git.openjdk.org/jdk/pull/30502#discussion_r3013810522
