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

[Alan Bateman]

On Wed, 14 Aug 2024 09:43:28 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8335771
>  - Review feedback
>  - Review feedback: avoid continue
>  - Review feedback
>  - 8335771

test/jdk/java/nio/channels/DatagramChannel/AfterDisconnect.java line 67:

> 65:      * may occur. In which case we want to retry the test.
> 66:      */
> 67:     public class BindExceptionOnDisconnect extends BindException {

I assume these interfaces don't need to be public.

test/jdk/java/nio/channels/DatagramChannel/AfterDisconnect.java line 127:

> 125:                 if (i < max -1 && retryOn.test(t)) {
> 126:                     System.out.println("Got " + t + "; will retry");
> 127:                 } else throw t;

What you would think about renaming this to "testWithRetry" so that it's clearer what it does? It could move up to the be the interface declarations so it is easier to find.

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

> 126:                 // To avoid this, when on macOS, we preemptively bind `dc` to the
> 127:                 // specific address instead of letting it bind to the wildcard.
> 128:                 if (Platform.isOSX()) {

Okay, but does feel like we are working around a macOS bug.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20050#discussion_r1718453128
PR Review Comment: https://git.openjdk.org/jdk/pull/20050#discussion_r1718455945
PR Review Comment: https://git.openjdk.org/jdk/pull/20050#discussion_r1718457881