On Wed, 4 Sep 2024 15:20:46 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Ravi Gupta has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8333403: Review comments fixed
>
> test/jdk/java/awt/event/ComponentEvent/ComponentEventTest.java line 161:
>
>> 159:
>> 160: robot.mouseMove(compAt.x + compSize.width / 2,
>> 161: compAt.y + compSize.height / 2);
>
> Suggestion:
>
> // Add a comment why you click a component
> EventQueue.invokeAndWait(() -> {
> Point location = components[9].getLocationOnScreen();
> Dimension size = components[9].getSize();
> centerPoint = new Point(location.x + size.width / 2,
> location.y + size.height / 2);
> });
> robot.mouseMove(centerPoint.x, centerPoint.y);
>
> You can calculate the center point where you click inside `invokeAndWait` and
> then assign the resulting value to `centerPoint` (rename `compAt` to
> `centerPoint` or another name). Remove `compSize`.
I also suggest removing the blank line between `mouseMove` and `mousePress` and
`mouseRelease`.
> test/jdk/java/awt/event/ComponentEvent/ComponentEventTest.java line 164:
>
>> 162:
>> 163: robot.mousePress(InputEvent.BUTTON1_DOWN_MASK);
>> 164: robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);
>
> Why do you click a component? You didn't answer the previous question.
If you expect anything to change in the UI state after you click, you should
add `robot.waitForIdle()`. Yes, you used `robot.setAutoWaitForIdle(true)`, so
robot with call `waitForIdle()` after each event generated. Alternatively, you
can add `robot.delay(DELAY)` to wait a little to guarantee what you expect has
happened before you continue with the test.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19521#discussion_r1744001163
PR Review Comment: https://git.openjdk.org/jdk/pull/19521#discussion_r1744007695
