RFR: 8306812: Open source several AWT Miscellaneous tests [v5]

[Tejesh R]

On Thu, 25 May 2023 16:29:13 GMT, Prasanta Sadhukhan <psadhukhan at 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