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

Prasanta Sadhukhan psadhukhan at openjdk.org
Thu May 25 16:46:04 UTC 2023


On Wed, 24 May 2023 03:45:50 GMT, Tejesh R <tr at openjdk.org> wrote:

>> Open source few AWT Miscellaneous ( Panel, Popup, robot and scrollbar) tests.
>
> 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..

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

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

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

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`

test/jdk/java/awt/Scrollbar/ScrollbarKeyControlTest.java line 62:

> 60:     }
> 61: 
> 62:     public void init() throws Exception {

you can move the contents into main

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

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1205762575
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1205764460
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1205768223
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1205770151
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1205771198
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1205774807
PR Review Comment: https://git.openjdk.org/jdk/pull/13828#discussion_r1205777846



More information about the client-libs-dev mailing list