On Thu, 25 May 2023 16:29:13 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org>
wrote:
>> Tejesh R has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Updated based on review comments
>
> test/jdk/java/awt/PopupMenu/PopupMenuStayOpen.java line 65:
>
>> 63: frame.setSize(300, 300);
>> 64: frame.setLocation(20, 300);
>> 65: frame.setLocationRelativeTo(null);
>
> it will override the previous setLocation so that can be removed..
Updated.
> test/jdk/java/awt/PopupMenu/PopupMenuStayOpen.java line 103:
>
>> 101: robot.delay(500);
>> 102:
>> 103: EventQueue.invokeAndWait(() -> {
>
> why is it under EDT? you can make the variable volatile and remove EDT
I have updated the variable, but still it has to be under EDT right? Since it
is shared.
> test/jdk/java/awt/PopupMenu/PopupMenuStayOpen.java line 118:
>
>> 116: }
>> 117:
>> 118: public static Point getLocation(Component co) throws
>> RuntimeException {
>
> I guess you can remove this method and use `frame.getLocationOnScreen` instead
Updated.
> test/jdk/java/awt/Robot/RobotMoveMultiscreen.java line 44:
>
>> 42: public class RobotMoveMultiscreen {
>> 43: static int x_dest = 20;
>> 44: static int y_dest = 20;
>
> these are accessed in 2 threads, you need to make it volatile
Updated.
> test/jdk/java/awt/Robot/RobotMoveMultiscreen.java line 79:
>
>> 77: robot.waitForIdle();
>> 78: robot.mouseMove(x_dest+50, y_dest+50);
>> 79: robot.delay(1000);
>
> I dont think we need to wait for 1sec after mouseMove, you can just have
> `waitForIdle`
Updated.
> test/jdk/java/awt/Scrollbar/ScrollbarKeyControlTest.java line 62:
>
>> 60: }
>> 61:
>> 62: public void init() throws Exception {
>
> you can move the contents into main
Means, should I make everything static and then change the Listener's
references?
> test/jdk/java/awt/Scrollbar/ScrollbarKeyControlTest.java line 84:
>
>> 82: });
>> 83: robot = new Robot();
>> 84: testOneScrollbar(scrollbarV);
>
> you should need to add robot.delay(1000) after frame is visible before
> commencing test
Updated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1206343117
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1206343821
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1206344028
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1206345886
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1206344201
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1206345515
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1206344479
