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