Re: RFR: 8213573: MouseLocationOnScreenTest fails intermittently [v2]

2020-10-23 Thread Kevin Rushforth
On Fri, 23 Oct 2020 13:57:46 GMT, Pankaj Bansal  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
>
> Pankaj Bansal has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixing review comments

Marked as reviewed by kcr (Lead).

-

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


Re: RFR: 8213573: MouseLocationOnScreenTest fails intermittently [v2]

2020-10-23 Thread Pankaj Bansal
On Fri, 23 Oct 2020 13:33:35 GMT, Kevin Rushforth  wrote:

>> Pankaj Bansal has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fixing review comments
>
> 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.

Fixed

-

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


Re: RFR: 8213573: MouseLocationOnScreenTest fails intermittently [v2]

2020-10-23 Thread Pankaj Bansal
> 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

Pankaj Bansal has updated the pull request incrementally with one additional 
commit since the last revision:

  Fixing review comments

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/333/files
  - new: https://git.openjdk.java.net/jfx/pull/333/files/4992e528..adc62f56

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=333=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=333=00-01

  Stats: 12 lines in 1 file changed: 0 ins; 8 del; 4 mod
  Patch: https://git.openjdk.java.net/jfx/pull/333.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/333/head:pull/333

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