RFR: JDK-8340555 : Open source DnD tests - Set4 [v8]

Alexey Ivanov aivanov at openjdk.org
Thu Oct 3 16:54:38 UTC 2024


On Wed, 2 Oct 2024 17:49:41 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> Following tests are added as part of this PR:
>> 
>> 1. /java/awt/dnd/DnDHTMLToOutlookTest/DnDHTMLToOutlookTest.java - **Manual PassFailJFrame (PFJ) test**, problemlisted on all platforms.
>> 
>> 2. java/awt/dnd/DragSourceMotionListenerTest.java -  **Automated test**, problemlisted on windows
>> 
>> 3. /java/awt/dnd/DragToAnotherScreenTest.java - **Manual PFJ test**
>> 
>> 4. /java/awt/dnd/RejectDragTest.java - **Automated test**, problemlisted on macOS
>
> Harshitha Onkar has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - unused import
>  - RejectDragTest

Looks good to me except for the formatting issue and a little simplification.

Yet I still want an answer about problem-listing. Do the updated tests still fail?

test/jdk/java/awt/dnd/DragSourceMotionListenerTest.java line 159:

> 157:                 p = getPoint(target, 1);
> 158:                 dstInsidePoint = p;
> 159:                 testPoint2.setLocation(p);

Now the local variable isn't needed any more, all the magic happens inside `getPoint`:

Suggestion:

                srcPoint = getPoint(source, 1);

                dstOutsidePoint = getPoint(frame, 3);
                testPoint1.setLocation(dstOutsidePoint);

                dstInsidePoint = getPoint(target, 1);
                testPoint2.setLocation(dstInsidePoint);


In fact, I meant getting rid of the lambda and moving its body into the helper method with the local variables as you wrote it and as I suggested to modify.

At the same time, this way also works: you moved out the common logic, and the lambda expression is not as long.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21213#pullrequestreview-2346114623
PR Review Comment: https://git.openjdk.org/jdk/pull/21213#discussion_r1786529997


More information about the client-libs-dev mailing list