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

Alexey Ivanov aivanov at openjdk.org
Mon Nov 18 19:58:01 UTC 2024


On Fri, 15 Nov 2024 21:04:10 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> Alisen Chung has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - EOF newline ActionEventTest
>>  - revert ActionEventTest
>
> 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;

Not required… because of `invokeAndWait`, yet it's a common practice to mark with `volatile`.

> 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.

Perhaps, it's better to use `getSystemLookAndFeelClassName()`.

> 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.

It don't think it's necessary or required. The test does not access UI at all, it just verifies the title of menu items.

Adding `delay(500)` to `sync` makes the test run longer… without a reason.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1847163245
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1847150967
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1847165866


More information about the client-libs-dev mailing list