On Mon, 9 Jun 2025 18:17:56 GMT, Anass Baya <[email protected]> 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 six additional
> commits since the last revision:
>
> - The bug id is not needed
>
> Co-authored-by: Abhishek Kumar <[email protected]>
> - No need to add bug id
>
> Co-authored-by: Abhishek Kumar <[email protected]>
> - Update test/jdk/java/awt/FileDialog/DoubleActionESC.java
>
> Co-authored-by: Abhishek Kumar <[email protected]>
> - Update test/jdk/java/awt/FileDialog/DoubleActionESC.java
>
> Co-authored-by: Abhishek Kumar <[email protected]>
> - Update test/jdk/java/awt/FileDialog/DoubleActionESC.java
>
> Co-authored-by: Abhishek Kumar <[email protected]>
> - Increased the latch timeout to ensure the test doesn't fail incorrectly.
LGTM.
test/jdk/java/awt/FileDialog/DoubleActionESC.java line 41:
> 39: /*
> 40: * @test
> 41: * @bug 5097243 8355478
No need to add bug id. We add the bug id only if there is any product change.
Suggestion:
* @bug 5097243
test/jdk/java/awt/FileDialog/DoubleActionESC.java line 89:
> 87:
> 88: if (!latch.await(LATCH_TIMEOUT, SECONDS))
> 89: {
Suggestion:
if (!latch.await(LATCH_TIMEOUT, SECONDS)) {
test/jdk/java/awt/FileDialog/DoubleActionESC.java line 90:
> 88: if (!latch.await(LATCH_TIMEOUT, SECONDS))
> 89: {
> 90: throw new RuntimeException("Test failed: Latch timout
> reached");
Suggestion:
throw new RuntimeException("Test failed: Latch timeout
reached");
test/jdk/java/awt/FileDialog/DoubleActionESC.java line 94:
> 92: EventQueue.invokeAndWait(() -> {
> 93: if (fd.isVisible())
> 94: {
Suggestion:
if (fd.isVisible()) {
test/jdk/java/awt/FileDialog/DoubleActionESCWithGtkDisabled.java line 26:
> 24: /*
> 25: * @test
> 26: * @bug 5097243 8355478
Don't think bug id is needed here as well.
Suggestion:
* @bug 5097243
-------------
Marked as reviewed by abhiscxk (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/25184#pullrequestreview-2882250966
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2116379043
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2116382966
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2116384670
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2116383654
PR Review Comment: https://git.openjdk.org/jdk/pull/25184#discussion_r2116436779
