Re: RFR: 8317287: [macos14] InterJVMGetDropSuccessTest.java: Child VM: abnormal termination [v9]
On Tue, 19 Dec 2023 02:09:08 GMT, songpv-imt 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 Changes requested by aivanov (Reviewer). test/jdk/java/awt/dnd/InterJVMGetDropSuccessTest/InterJVMGetDropSuccessTest.java line 1: > 1: /* GitHub doesn't allow leaving comments to unmodified lines therefore I'm leaving a file comment: 1. Please organise imports: use per-class imports instead of wildcard imports. 2. Use Java-style array declaration for `successCodes` and declare it as `final` to resolve IDE warnings. 3. Remove unused `successCodes` field and `reset` method. 4. Remove unused `pointInComponent` and `pointInComponentImpl` methods. - PR Review: https://git.openjdk.org/jdk/pull/16396#pullrequestreview-1822311145 PR Review Comment: https://git.openjdk.org/jdk/pull/16396#discussion_r1452779145
Re: RFR: 8317287: [macos14] InterJVMGetDropSuccessTest.java: Child VM: abnormal termination [v9]
On Tue, 19 Dec 2023 02:09:08 GMT, songpv-imt 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
Please update the copyright year in both files to 2024.
test/jdk/java/awt/dnd/InterJVMGetDropSuccessTest/InterJVMGetDropSuccessTest.java
line 301:
> 299: for (Point p = new Point(sourcePoint);
> !p.equals(targetPoint);
> 300: p.translate(Util.sign(targetPoint.x - p.x),
> 301: Util.sign(targetPoint.y - p.y))) {
Suggestion:
p.translate(Util.sign(targetPoint.x - p.x),
Util.sign(targetPoint.y - p.y))) {
Align the second parameter on the wrapped line to the first one on the first
line.
-
Changes requested by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/16396#pullrequestreview-1822259995
PR Review Comment: https://git.openjdk.org/jdk/pull/16396#discussion_r1452746985
Re: RFR: 8317287: [macos14] InterJVMGetDropSuccessTest.java: Child VM: abnormal termination [v9]
On Tue, 19 Dec 2023 02:09:08 GMT, songpv-imt 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
Re: RFR: 8317287: [macos14] InterJVMGetDropSuccessTest.java: Child VM: abnormal termination [v9]
> 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 - Changes: - all: https://git.openjdk.org/jdk/pull/16396/files - new: https://git.openjdk.org/jdk/pull/16396/files/2521a8e0..83ad763b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16396=08 - incr: https://webrevs.openjdk.org/?repo=jdk=16396=07-08 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16396.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16396/head:pull/16396 PR: https://git.openjdk.org/jdk/pull/16396
