RFR: 8335771: Improve stability of java/nio/channels/DatagramChannel tests [v2]

Alan Bateman alanb at openjdk.org
Tue Jul 16 14:29:54 UTC 2024


On Wed, 10 Jul 2024 20:09:58 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Several `java/nio/channels/DatagramChannel` tests fail intermittently. Though it seems difficult to fix all intermittent failures due to the nature of what some of these attempt to test, the stability can still be improved.
>> 
>> The most common source of intermittent failures are:
>> 
>>  - not receiving a datagram due to port conflict or interference of concurrent running tests
>>  - failure to disconnect due to interference of concurrent running tests
>>  - failure due to reception of unexpected stray datagrams
>> 
>> Avoiding to bind on the wildcard address, filtering unexpected datagram, and some retrying can aleviate some of the issues and greatly reduce the frequency of intermittent failures.
>
> Daniel Fuchs has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Review feedback: avoid continue
>  - Review feedback

test/jdk/java/nio/channels/DatagramChannel/AdaptorMulticasting.java line 433:

> 431:             p = new DatagramPacket(new byte[1024], 100);
> 432:             s.receive(p);
> 433:             if (getPort(p.getSocketAddress()) == getPort(s.getLocalSocketAddress())) {

I assume this can be simplified to `if (p.getPort() == s.getLocalPort())`, meaning no need to use SocketAddress here.

test/jdk/java/nio/channels/DatagramChannel/AdaptorMulticasting.java line 445:

> 443:                 System.out.println("Received message sender doesn't match - skipping: " + p.getSocketAddress());
> 444:             }
> 445:         } while(true);

`do { ... } while(true);` looks a bit odd here, any reason now to use `while (true) { .. }` ?

test/jdk/java/nio/channels/DatagramChannel/Connect.java line 121:

> 119:                 bb.put(bytes);
> 120:                 bb.flip();
> 121:                 if (Platform.isOSX()) {

If we look at this test next year then we will wonder why the bind is macOS specific, I think this needs a comment.

test/jdk/java/nio/channels/DatagramChannel/NotBound.java line 112:

> 110:         dc = DatagramChannel.open();
> 111:         try {
> 112:             System.out.println("Checks that connect() binds the socket");

I assume the trace message should be "Check" rather than "Checks" here (same thing for the other messages)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20050#discussion_r1679510532
PR Review Comment: https://git.openjdk.org/jdk/pull/20050#discussion_r1679512922
PR Review Comment: https://git.openjdk.org/jdk/pull/20050#discussion_r1679515830
PR Review Comment: https://git.openjdk.org/jdk/pull/20050#discussion_r1679519642


More information about the nio-dev mailing list