RFR: 8355478: DoubleActionESC.java fails intermittently [v2]

[Alexey Ivanov]

On Thu, 15 May 2025 13:51:07 GMT, Anass Baya <abaya at openjdk.org> wrote:

>> **Analysis :**
>> 
>> Whether the test passes on the main line or fails, the behavior is still incorrect.
>> This test is meant to ensure that pressing ESC a second time while the file dialog is open behaves correctly.
>> 
>> However, the CountDownLatch is currently set to 1, which means the test only waits for the first open/close interaction to complete. As a result, it does not wait for the second attempt (opening the dialog again and pressing ESC to close it), because the latch reaches zero after the first attempt.
>> 
>> This causes the test to proceed immediately to the validation step:
>> 
>> if (fd.isVisible()) {
>>     throw new RuntimeException("File Dialog is not closed");
>> }
>> 
>> At this point, whether the test passes or fails becomes unreliable and undefined, as it depends on the state of the second attempt (whether the file dialog is in the process of opening, being closed, or hasn't even started yet)
>> 
>> To ensure the test behaves correctly, the CountDownLatch should be set to 2, so it waits for the two attempt of open-close interactions to be completed before moving on to validation.
>> 
>> **Proposed fix:**
>> 
>> set the CountDownLatch to 2
>> 
>> **Proposed enhancements :**
>> 
>> Remove unnecessary threads (Thread and Thread-2)
>> Properly handle delays and robot.waitForIdle()
>> Avoid indefinite blocking on latch.await()
>> 
>> With these enhancements, the test execution time is reduced from around 3 minutes to approximately 1 minute 30 seconds
>> 
>> The adapted test uncovered a new bug in GTKFileDialog on Linux, which is being tracked under [JDK-8356981](https://bugs.openjdk.org/browse/JDK-8356981). As a result, it has been added to the ProblemList. See [JDK-8356981](https://bugs.openjdk.org/browse/JDK-8356981) for more details
>
> Anass Baya has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update test/jdk/ProblemList.txt
>   
>   Co-authored-by: Abhishek Kumar <abhishek.cx.kumar at oracle.com>

A sanity check: does the updated test reproduce the problem reported in [JDK-5097243](https://bugs.openjdk.org/browse/JDK-5097243)? I'm pretty sure it should.

test/jdk/java/awt/FileDialog/DoubleActionESC.java line 56:

> 54:     private static volatile CountDownLatch latch;
> 55:     private static final int REPEAT_COUNT = 2;
> 56:     private static final long LATCH_TIMEOUT = 4;

Suggestion:

    private static final int REPEAT_COUNT = 2;
    private static final long LATCH_TIMEOUT = 4;

        private static final CountDownLatch latch = new CountDownLatch(REPEAT_COUNT);

test/jdk/java/awt/FileDialog/DoubleActionESC.java line 81:

> 79:                 robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);
> 80:                 robot.waitForIdle();
> 81:                 robot.delay(1000);

Can the delay be reduced to `500`?

test/jdk/java/awt/FileDialog/DoubleActionESC.java line 89:

> 87:             }
> 88: 
> 89:             latch.await(LATCH_TIMEOUT, SECONDS);

Throw an exception if the timeout is reached — the test already failed.

test/jdk/java/awt/FileDialog/DoubleActionESC.java line 90:

> 88: 
> 89:             latch.await(LATCH_TIMEOUT, SECONDS);
> 90:             if (fd.isVisible()) {

I'm on the fence here… Calling `fd.isVisible()` on EDT is more reliable; but it's not strictly required for AWT components.

test/jdk/java/awt/FileDialog/DoubleActionESC.java line 92:

> 90:             if (fd.isVisible()) {
> 91:                 throw new RuntimeException("File Dialog is not closed");
> 92:             }

Dispose of `fd` too (in the finally block).

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25184#pullrequestreview-2844332500
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2091558746
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2091564010
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2091576832
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2091578738
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2091575682