On Thu, 22 Oct 2020 14:22:32 GMT, Pankaj Bansal <pban...@openjdk.org> wrote:

> The test test.robot.javafx.scene.MouseLocationOnScreenTest fails 
> intermittently on Windows and Mac system though it fails on Windows more 
> often than Mac.
> 
> The test uses robot to move the mouse at different positions on screen using 
> mouseMove and then validates the mouse coordinates by robot.getMouseX/Y. If 
> the coordinates returned from Robot.getMouseX/Y are same as mouse coordinates 
> passed to Robot.mouseMove, the test passes, else the test fails.
> 
> The issue here is that there is no wait/delay between the calls to mouseMove 
> and getMouseX/Y. When Robot.mouseMove is used to move mouse to a coordinate, 
> robot in turn sends the event to native system. When Robot.getMouseX/Y is 
> called to get the current mouse coordinates, robot again uses native 
> functions to get the mouse location. In current test, it is possible that the 
> Robot.mouseMove is called with particular x,y coordinates and which in turn 
> has send the events to native. As there is no delay/wait after calling 
> Robot.mouseMove, When the Robot.getMouseX/Y is called, it is possible that 
> the earlier event sent to native has not been processed yet. So the mouse has 
> not been moved and Robot.getMouseX/Y may return older mouse coordinates. This 
> results in the test to fail. 
> 
> In short, the Robot.mouseMove is an asynchronous call, so it should be used 
> in same way. A follow up bug will be filled to update the specification to 
> reflect this. The test is assuming it to be synchronous and results in 
> intermittent failures.
> 
> The fix is to add small delay after calling Robot.mouseMove. Along with this, 
> now as the test will take more time, so the timeout time for test is 
> increased to 120s. Also, I have divided the calls to edge(...) among more 
> Util.runAndWait calls as there is a timeout limit for Util.runAndWait which 
> is set to 10s.
> 
> I tested this on Windows 10, Ubuntu 18.04, Ubuntu 20.04, Oracle Linux 8.2, 
> Mac 10.13. For me this test is passing in less than 50 seconds on all 
> platforms. Following command can be used to run the test
> gradle --continue --info -PFULL_TEST=true -PUSE_ROBOT=true 
> :systemTest:cleanTest :systemTests:test --tests 
> test.robot.javafx.scene.MouseLocationOnScreenTest

The fix looks good to me with a couple minor comments. I ran it on the 
following systems:

* Windows 10
* macOS 10.15.7
* macOS 11 beta9
* Ubuntu 16.04
* Ubuntu 20.04 (VM on Win 10 host)
* Oracle Linux 7.7 (VM on Win 10 host)

The updated test passed on all systems. On the slowest system, the test took 
about 45 seconds for most runs and was always under 50 seconds.

tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java
 line 138:

> 136:             for (int y = y1; y <= y2; y++) {
> 137:                 robot.mouseMove(x, y);
> 138:                 delay(delayTime);

Minor: you can use `Util.sleep` here instead of creating your own method.

tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java
 line 166:

> 164:             Thread.sleep(time);
> 165:         } catch (InterruptedException e) {
> 166: 

Minor: there is an extra blank line here, although if you use `Util.sleep` 
instead you can remove this method entirely.

tests/system/src/test/java/test/robot/javafx/scene/MouseLocationOnScreenTest.java
 line 46:

> 44:     static CountDownLatch startupLatch;
> 45:     static Robot robot;
> 46:     private static int delayTime = 1;

Minor: typically such constants are all upper case with words separated by `_`.

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

PR: https://git.openjdk.java.net/jfx/pull/333

Reply via email to