RFR: 8339524: Clean up a few ExtendedRobot tests [v6]

Harshitha Onkar honkar at openjdk.org
Fri Nov 15 21:28:12 UTC 2024


On Fri, 15 Nov 2024 14:35:25 GMT, Alisen Chung <achung at openjdk.org> wrote:

>> Cleaning up tests building ExtendedRobot that shouldn't be.
>
> Alisen Chung has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - EOF newline ActionEventTest
>  - revert ActionEventTest

LGTM apart from the following minor inline comments. 

Since @aivanov-jdk reviewed previously please wait to see in case he wants to add anything additional.

test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 49:

> 47:     private JInternalFrame iFrame;
> 48:     private static TestTitlePane testTitlePane;
> 49:     private boolean passed;

Suggestion:

    private static volatile boolean passed;

test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 50:

> 48:     private static TestTitlePane testTitlePane;
> 49:     private boolean passed;
> 50:     private static final Robot robot = createRobot();

Suggestion but not a must change: Since createRobot() is just creating an instance of Robot, the extra method can be removed and robot can be initialized in main method.

test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 54:

> 52:     public static void main(String[] args) throws Exception {
> 53:         UIManager.setLookAndFeel(
> 54:                 new com.sun.java.swing.plaf.windows.WindowsClassicLookAndFeel());

Is fully qualified name required? I think we can shorten it by adding required imports.

test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 138:

> 136: 
> 137:     private static void sync() {
> 138:         robot.waitForIdle();

Since sync() is called after UI setup better to add `robot.delay(500)` for stability.

test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 146:

> 144:         } catch (Exception ex) {
> 145:             throw new Error("Can't create Robot", ex);
> 146:         }

Same with com.sun.java.swing.plaf.windows.WindowsInternalFrameTitlePane , fully qualified name can be replaced.

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

Marked as reviewed by honkar (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20846#pullrequestreview-2439644547
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844486162
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844487898
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844481698
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844482837
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844483763


More information about the client-libs-dev mailing list