JDK-8213573 : MouseLocationOnScreenTest fails intermittently
  • Type: Bug
  • Component: javafx
  • Sub-Component: window-toolkit
  • Affected Version: openjfx11
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-11-09
  • Updated: 2022-11-08
  • Resolved: 2020-10-23
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
openjfx16Fixed
Related Reports
Relates :  
JDK-8196031 - FX Robot mouseMove fails on Windows 10 1709 with HiDPI
Relates :  
JDK-8296554 - MouseLocationOnScreenTest sometime fails when system is busy
Description
To reproduce, run the system tests on a system running Windows 10 version 1803 with Hi-DPI scaling at 125% as follows:

$ gradle -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests MouseLocationOnScreenTest

test.robot.javafx.scene.MouseLocationOnScreenTest > testMouseLocation FAILED
    java.lang.AssertionError: expected:<54> but was:<53>
        at org.junit.Assert.fail(Assert.java:91)
        at org.junit.Assert.failNotEquals(Assert.java:645)
        at org.junit.Assert.assertEquals(Assert.java:126)
        at org.junit.Assert.assertEquals(Assert.java:470)
        at org.junit.Assert.assertEquals(Assert.java:454)
        at test.robot.javafx.scene.MouseLocationOnScreenTest.validate(MouseLocationOnScreenTest.java:115)
        at test.robot.javafx.scene.MouseLocationOnScreenTest.edge(MouseLocationOnScreenTest.java:123)
        at test.robot.javafx.scene.MouseLocationOnScreenTest.lambda$testMouseLocation$1(MouseLocationOnScreenTest.java:83)

The exact point at which it fails will differ depending on where the mouse is on the screen when the tests starts running. The failure mode suggests some sort of rounding error or similar product bug in the mouse move code, which was modified to work on Windows 10 and now seems to have some slight inaccuracies with a later version.
Comments
Changeset: 2c675557 Author: Pankaj Bansal <pbansal@openjdk.org> Committer: Kevin Rushforth <kcr@openjdk.org> Date: 2020-10-23 14:12:50 +0000 URL: https://git.openjdk.java.net/jfx/commit/2c675557
23-10-2020
Updated the title to add that this is an intermittent failure and remove the platform-specific part, since it fails intermittently on Mac and non-HiDPI Windows, too.
20-10-2020
I sometimes see this fail on my new MacBookPro as well. After some offline discussion we have agreed that this a test bug, and will fix it by adding a small delay in the test. This will need to be done in such a way that it doesn't cause the test to run for too long. The best long term solution isto implement Robot::waitForIdle as described in JDK-8176902, but that is lot of effort and we are unlikely to do it. A new follow-on bug should be filed to change the spec to clarify that Robot::mouseMove is asynchronous and that Robot::getMousePosition will return the position at the time it is called, without respect to whether there are outstanding moseMove calls.
16-10-2020
I wasn't speaking of whether the implementation currently guarantees that a mouse move followed by a get mouse position would return the just-moved-to position. Clearly it doesn't. I was saying that it *SHOULD* make that guarantee. Meaning that the implementation of get mouse position ought to return the correct value even if the immediately preceding call was a mouse move. So I do think this is a bug in the implementation of mouseMove and/or getMousePosition (i.e., a bug in the Robot implementation on Windows, not a test bug) that causes the result of get to sometimes get the old position. I hope there is a way for getMousePosition to flush or otherwise make the calls synchronous. If not, then we might have to look at changing the spec to indicate that these operations can be asynchronous. That would not be my preferred option. Do we have this same limitation w.r.t. mouse move followed by mouse press? I presume not, since we haven't had seen any problems, but it would be worth knowing. As for a workaround, I suspect a sleep of 1ms would be (more than) sufficient. I'm less convinced that we ought to implement that workaround, and wouldn't want to do that without a better understanding of what our options are.
22-06-2020
.<<The implementation of the Robot methods mouseMove, getMouseX, and getMouseY should ensure that a call to mouseMove immediately followed by a call to getMouse{X,Y} will return the correct value without the need for a delay. I think this is not true. The mouseMove is sending an event/input to windows API which adds the event to Windows mouse/keyboard event stream/queue. There may be some other event in the queue or it may take some time to process the sent event. Meanwhile, if we call getMouse{x,y}, the last event/input sent may have not been processed by Windows API. So the mouse cursor may be at old position. So we get the old value. I have tested this and found this to be true. When ever the test fails, the value from getMouse{x,y} is same as the value from last getMouse{x,y} call. This means that the Windows could not process the last event sent and could not move the cursor to new location before we inquired the value. Also, the test was taking 10 min as I had put 50 ms as wait time after every mousemove. I can see that even if we add a delay of 2 ms, the test is always passing on my machine. The test passes in less than 1 minute. I have tested it with all HiDPI scales multiple times and it has passed without any exception. But I think, the delay will depend upon the machine speed and to make the test stable, we will have to add a higher delay that 2 ms. Following is the patch with 2 ms and this always passes in less than 1 minute. Please note that, this is in no way a final or close to final change. This is just to demonstrate the the issue . diff --git a/tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java b/tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java index 765a8aea49..ec8862ef7a 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java @@ -43,6 +43,7 @@ import test.util.Util; public class MouseLocationOnScreenTest { static CountDownLatch startupLatch; static Robot robot; + static int delayTime = 2; public static class TestApp extends Application { @@ -68,7 +69,7 @@ public class MouseLocationOnScreenTest { } } - @Test(timeout = 20000) + @Test(timeout=1000000) public void testMouseLocation() throws Exception { Screen screen = Screen.getPrimary(); @@ -81,23 +82,37 @@ public class MouseLocationOnScreenTest { // Check all edge (two pixels in a width) Util.runAndWait(() -> { edge(robot, x1, y1, x2, y1); // top + }); + Util.runAndWait(() -> { edge(robot, x1, y1 + 1, x2, y1 + 1); // top + }); + Util.runAndWait(() -> { edge(robot, x2, y1, x2, y2); // right + }); + Util.runAndWait(() -> { edge(robot, x2 - 1, y1, x2 - 1, y2); // right }); Util.runAndWait(() -> { edge(robot, x1, y1, x1, y2); // left + }); + Util.runAndWait(() -> { edge(robot, x1 + 1, y1, x1 + 1, y2); // left + }); + Util.runAndWait(() -> { edge(robot, x1, y2, x2, y2); // bottom + }); + Util.runAndWait(() -> { edge(robot, x1, y2 - 1, x2, y2 - 1); // bottom }); // Check crossing of diagonals Util.runAndWait(() -> { cross(robot, x1, y1, x2, y2); // cross left-bottom + }); + Util.runAndWait(() -> { cross(robot, x1, y2, x2, y1); // cross left-top }); } @@ -119,7 +134,9 @@ public class MouseLocationOnScreenTest { private static void edge(Robot robot, int x1, int y1, int x2, int y2) { for (int x = x1; x <= x2; x++) { for (int y = y1; y <= y2; y++) { + System.out.println("x: "+ x + "y: " + y); robot.mouseMove(x, y); + delay(delayTime); validate(robot, x, y); } } @@ -131,13 +148,22 @@ public class MouseLocationOnScreenTest { double dy = (y1 - y0) / dmax; robot.mouseMove(x0, y0); + delay(delayTime); validate(robot, x0, y0); for (int i = 1; i <= dmax; i++) { int x = (int) (x0 + dx * i); int y = (int) (y0 + dy * i); robot.mouseMove(x, y); + delay(delayTime); validate(robot, x, y); } } + + private static void delay(int time) { + try { + Thread.sleep(time); + } catch (InterruptedException e) { + } + } }
22-06-2020
That's an interesting finding. The patch isn't really a feasible solution: having this single test take 10 minutes isn't really practical. It might lead to a better understanding of the problem, though. Based on what you've found so far, I don't agree that this is a test issue. The implementation of the Robot methods mouseMove, getMouseX, and getMouseY should ensure that a call to mouseMove immediately followed by a call to getMouse{X,Y} will return the correct value without the need for a delay. Adding a delay to the test would seem to be a workaround for a bug in Robot as opposed to a fix for a bug in the test. Can you check the underlying native functions we are calling on Windows and see if we might be missing a flush or synchronization call?
19-06-2020
This is a test issue. There is no delay after mousemove before validating the mouse coordinates. This is test is not specific to a particular windows version or HiDPI setting. I am able to reproduce this issue with Windows 10 Version 1909 with HiDPI scale 100%, 125%, 150% and 175%. As it is delay issue, the test fails at any place randomly. I have managed to make this test work by introducing the delay after each mousemove before validating the coordinates. Due to addition of delay after every mousemove, the test is taking almost 10 minutes to complete. Below is the patch which makes the test pass. I have tested this with HiDPI scale of 100%, 125%, 150% and 175% and test passes for me always. I have increased the timeout for util->runAndWait as default timeout for this is 10s and after addition of delay, even one row can not be completed in 10s. There are temporary changes as I was not aware how to increase timeout of util->runAndWait by any other way. This is just a way to demonstrate that the test can be fixed by adding proper delays. diff --git a/tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java b/tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java index 765a8aea49..b1d3f7b4a0 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java @@ -68,7 +68,7 @@ public class MouseLocationOnScreenTest { } } - @Test(timeout = 20000) + @Test(timeout=1000000) public void testMouseLocation() throws Exception { Screen screen = Screen.getPrimary(); @@ -81,23 +81,37 @@ public class MouseLocationOnScreenTest { // Check all edge (two pixels in a width) Util.runAndWait(() -> { edge(robot, x1, y1, x2, y1); // top + }); + Util.runAndWait(() -> { edge(robot, x1, y1 + 1, x2, y1 + 1); // top + }); + Util.runAndWait(() -> { edge(robot, x2, y1, x2, y2); // right + }); + Util.runAndWait(() -> { edge(robot, x2 - 1, y1, x2 - 1, y2); // right }); Util.runAndWait(() -> { edge(robot, x1, y1, x1, y2); // left + }); + Util.runAndWait(() -> { edge(robot, x1 + 1, y1, x1 + 1, y2); // left + }); + Util.runAndWait(() -> { edge(robot, x1, y2, x2, y2); // bottom + }); + Util.runAndWait(() -> { edge(robot, x1, y2 - 1, x2, y2 - 1); // bottom }); // Check crossing of diagonals Util.runAndWait(() -> { cross(robot, x1, y1, x2, y2); // cross left-bottom + }); + Util.runAndWait(() -> { cross(robot, x1, y2, x2, y1); // cross left-top }); } @@ -120,6 +134,7 @@ public class MouseLocationOnScreenTest { for (int x = x1; x <= x2; x++) { for (int y = y1; y <= y2; y++) { robot.mouseMove(x, y); + delay(50); validate(robot, x, y); } } @@ -131,13 +146,22 @@ public class MouseLocationOnScreenTest { double dy = (y1 - y0) / dmax; robot.mouseMove(x0, y0); + delay(50); validate(robot, x0, y0); for (int i = 1; i <= dmax; i++) { int x = (int) (x0 + dx * i); int y = (int) (y0 + dy * i); robot.mouseMove(x, y); + delay(50); validate(robot, x, y); } } + + private static void delay(int time) { + try { + Thread.sleep(time); + } catch (InterruptedException e) { + } + } } diff --git a/tests/system/src/test/java/test/util/Util.java b/tests/system/src/test/java/test/util/Util.java index 5616bfc078..29a2a956e7 100644 --- a/tests/system/src/test/java/test/util/Util.java +++ b/tests/system/src/test/java/test/util/Util.java @@ -47,7 +47,7 @@ import org.junit.Assert; public class Util { // Test timeout value in milliseconds - public static final int TIMEOUT = 10000; + public static final int TIMEOUT = 150000; private static interface Future { public abstract boolean await(long timeout, TimeUnit unit);
19-06-2020
I ran this test in Window 10 Version 1909 and I am able to reproduce this there as well Also, I see that issue is reproducible even without HiDPI as I am able to see the issue with 100% scale. Following is the log for HiDPI scale 100% test.robot.javafx.scene.MouseLocationOnScreenTest > testMouseLocation FAILE D java.lang.AssertionError: expected:<261> but was:<260> at org.junit.Assert.fail(Assert.java:91) at org.junit.Assert.failNotEquals(Assert.java:645) at org.junit.Assert.assertEquals(Assert.java:126) at org.junit.Assert.assertEquals(Assert.java:470) at org.junit.Assert.assertEquals(Assert.java:454) at test.robot.javafx.scene.MouseLocationOnScreenTest.validate(MouseLocat ionOnScreenTest.java:115) at test.robot.javafx.scene.MouseLocationOnScreenTest.edge(MouseLocationO nScreenTest.java:123) at test.robot.javafx.scene.MouseLocationOnScreenTest.lambda$testMouseLoc ation$1(MouseLocationOnScreenTest.java:83)
18-06-2020