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

Alexey Ivanov aivanov at openjdk.org
Thu Oct 24 14:37:12 UTC 2024


On Tue, 22 Oct 2024 20:29:45 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 one additional commit since the last revision:
> 
>   update tests

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/TrayIcon/ActionEventTest/ActionEventTest.java line 30:

> 28:  * @summary Verify that ActionEvent is received with correct modifiers set.
> 29:  * @modules java.desktop/java.awt:open java.desktop/java.awt.peer
> 30:  * @library /lib/client ../ /java/awt/patchlib

Is anything else but `ExtendedRobot` used from `/lib/client`? If not, remove it.

test/jdk/java/awt/TrayIcon/ActionEventTest/ActionEventTest.java line 54:

> 52:         if (!SystemTray.isSupported()) {
> 53:             System.out.println("SystemTray not supported on the platform." +
> 54:                     " Marking the test passed.");

Perhaps, throwing `jtreg.SkippedException` is a better option.

Now skipped tests are distinguished from passed ones.

test/jdk/java/awt/TrayIcon/ActionEventTest/ActionEventTest.java line 65:

> 63:                                 "all icons and notifications on the taskbar\" true " +
> 64:                                 "to avoid this problem.\nOR change behavior only for " +
> 65:                                 "Java SE tray icon and rerun test.");

~~I'd rather keep them aligned as they are — easier to read.~~

Shall we remove this at all?

The test is automatic, isn't it? No one will see these messages unless the test fails. [The instructions](https://wiki.openjdk.org/display/ClientLibs/Automated+client+GUI+testing+system+set+up+requirements#AutomatedclientGUItestingsystemsetuprequirements-Windows-SpecificSystemsetupnotes) for setting up Windows hosts to run client tests include:

> Show all icons and notifications in taskbar and don't hide them  
> Right Click on Taskbar → Properties → (Notification area)  
> Click Customize → Enable checkbox "Always show all icons and notifications on the taskbar"

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

> 29:  * @summary Checks that JInternalFrame's system menu
> 30:  *          can be localized during run-time
> 31:  * @library /lib/client/

Does the test use anything except `ExtendedRobot`? If not, remove `@library`.

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

> 55:             UIManager.setLookAndFeel(
> 56:                     new com.sun.java.swing.plaf.windows.WindowsClassicLookAndFeel());
> 57:         } catch (UnsupportedLookAndFeelException e) {

I would let the exception fail the test if it's run on anything but Windows.

The test has `@requires (os.family == "windows")`.

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

> 62:         final bug6725409 bug6725409 = new bug6725409();
> 63:         try {
> 64:             SwingUtilities.invokeAndWait(() -> bug6725409.setupUIStep1());

Suggestion:

            SwingUtilities.invokeAndWait(bug6725409::setupUIStep1);

Since you're still updating these lines, I'm for using method references (in all the cases).

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

> 84:         JDesktopPane desktop = new JDesktopPane();
> 85:         iFrame = new JInternalFrame("Internal Frame",
> 86:                 true, true, true, true);

The line fits in 80 columns, so I'd rather keep it untouched.

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

> 149:         try {
> 150:             Robot robot = new Robot();
> 151:             return robot;

Suggestion:

            return new Robot();

Can be inlined?

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

> 152:         } catch (Exception ex) {
> 153:             // pass exception
> 154:         }

This is wrong — if the test can't create `Robot`, let it fail right away.

Suggestion:

        } catch (Exception ex) {
            throw new Error("Can't create Robot", ex);
        }

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

PR Review: https://git.openjdk.org/jdk/pull/20846#pullrequestreview-2392728149
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815129228
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815075164
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815070228
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815116294
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815091498
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815093034
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815097951
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815109763
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1815106557


More information about the client-libs-dev mailing list