On Wed, 18 Jun 2025 18:38:45 GMT, Damon Nguyen <dngu...@openjdk.org> wrote:

>> This change is to restore the original intent of the test by updating the 
>> instructions to check that the type of Cursor is preserved when clicked and 
>> dragged. Now the test correctly has instructions to check that an I-beam 
>> cursor stays an I-beam until released over a List with its cursor being 
>> updated to a Hand cursor.
>> 
>> There is a bug where this does not correctly update in macOS (found in 
>> [JDK-7177297](https://bugs.openjdk.org/browse/JDK-7177297)). So, this test 
>> needs to be problem-listed.
>> 
>> I have confirmed that preserving the cursor image when dragging is native 
>> behavior across macOS, Windows, and Ubuntu. And I have checked that the test 
>> passes on both Windows and Ubuntu, while macOS fails and immediately updates 
>> the cursor as it leaves the TextArea.
>
> Damon Nguyen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Change timeout and instructions columns.

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/Cursor/CursorDragTest/ListDragCursor.java line 46:

> 44:     static Frame instructionsFrame;
> 45:     private static final CountDownLatch countDownLatch = new 
> CountDownLatch(1);
> 46:     static String INSTRUCTIONS = """

I'd place the fields in this order:


INSTRUCTIONS

testFrame
instructionsFrame

countDownLatch


The blank lines are explicit for the code, too. This way the fields come in 
groups: the instructions describe what the tester will do, so it's reasonable 
to put them at the top, and this aligns to the most common way in manual tests; 
then go the two frames of the test; the latch which controls the test follows.

Personally, I'd also declare all of them `private`.

test/jdk/java/awt/Cursor/CursorDragTest/ListDragCursor.java line 55:

> 53:                 and should stay the same while dragging across the
> 54:                 components. Once you reach the list, release the
> 55:                 left mouse button. Immediately after, the cursor

~~Immediately after~~ As soon as you release the left mouse button, the cursor…

test/jdk/java/awt/Cursor/CursorDragTest/ListDragCursor.java line 114:

> 112:         Panel btnPanel = new Panel();
> 113:         Button passBtn = new Button("PASS");
> 114:         Button failBtn = new Button("FAIL");

Suggestion:

        Button passBtn = new Button("Pass");
        Button failBtn = new Button("Fail");

Title case is good enough.

test/jdk/java/awt/Cursor/CursorDragTest/ListDragCursor.java line 123:

> 121:         failBtn.addActionListener(e -> {
> 122:             throw new RuntimeException("Test Failed");
> 123:         });

You have to release the latch, otherwise the test doesn't exit if it's run 
without jtreg.

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

PR Review: https://git.openjdk.org/jdk/pull/25705#pullrequestreview-2940332720
PR Review Comment: https://git.openjdk.org/jdk/pull/25705#discussion_r2155279444
PR Review Comment: https://git.openjdk.org/jdk/pull/25705#discussion_r2155281655
PR Review Comment: https://git.openjdk.org/jdk/pull/25705#discussion_r2155286080
PR Review Comment: https://git.openjdk.org/jdk/pull/25705#discussion_r2155289355

Reply via email to