On Tue, 19 Dec 2023 02:09:08 GMT, songpv-imt <d...@openjdk.org> wrote:

>> The root cause of the bug is because mousePress() method is invoked before 
>> mouseMove() event is completely processed causing the drag & drop behavior 
>> not being able to be recognized properly. This in turn makes the method 
>> dragSourceListener.isDropFinished() returns false and fail the test. To fix 
>> this, setAutoWaitForIdle(true) and Thread.Sleep is called to make sure the 
>> mouseMove() event is processed completely before moving to execute the 
>> mousePress() method.
>> 
>> JBS issue: [JDK-8317287](https://bugs.openjdk.org/browse/JDK-8317287)
>
> songpv-imt has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update InterJVMGetDropSuccessTest.java
>   - Update success1 and success2 to volatile

I reproduced the failure on macOS 14.1, drag isn't started. With the fix to the 
test, drag is started as expected, and the test passes successfully.

Yet I'd like to understand the test logic better. I think the test could be 
improved further, however, lots of refactoring could change the test logic.

A couple of points that can be improved though. Instead of using 
`robot.setWaitForIdle(true)`, it should be enough to add `robot.waitForIdle()` 
after `mouseMove`. This way, the test should execute quicker since there'll be 
no implicit `waitForIdle` between each event generated by robot. I believe 
`robot.delay(50)` could be removed from the for-loop which moves the mouse.

What I suggest is:


            Robot robot = new Robot();

            robot.mouseMove(sourcePoint.x, sourcePoint.y);
            robot.waitForIdle();
            robot.delay(50);
            robot.mousePress(InputEvent.BUTTON1_DOWN_MASK);
            for (Point p = new Point(sourcePoint); !p.equals(targetPoint);
                p.translate(Util.sign(targetPoint.x - p.x),
                             Util.sign(targetPoint.y - p.y))) {
                robot.mouseMove(p.x, p.y);
            }


The initial FRAME_ACTIVATION_TIMEOUT could also be reduced to 1_000 with an 
additional `robot.waitForIdle()`.

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

PR Comment: https://git.openjdk.org/jdk/pull/16396#issuecomment-1892769787

Reply via email to