RFR: 8354219 : Automate javax/swing/JComboBox/ComboPopupBug.java

[Alexey Ivanov]

On Mon, 14 Apr 2025 13:50:54 GMT, Anass Baya <abaya at openjdk.org> wrote:

> This test was designed to manually verify that clicking on the JComboBox when the frame containing it is about to close does not cause an IllegalStateException.
> 
> The test allowed the tester extra time to click on the JComboBox when closing the frame by adding a Thread.sleep() in the close button handler.
> 
> In this test, a JComboBox is displayed with a Close button at the bottom. The tester should click the Close button, then try to click the JComboBox arrow button to display the popup.
> 
> In the automated test, we save the JComboBox  location size before closing the frame. We then use this information to click on the JComboBox right before the frame is closed.

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 1:

> 1: /*

Please update the copyright year to `2024, 2025,`

test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 51:

> 49: 
> 50:     public static void main(String[] args) throws Exception {
> 51:         robot = JRobot.getRobot();

You don't use any of `JRobot` features, just use the `Robot` class: `new Robot()`.

test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 71:

> 69:             frame.setSize(200, 200);
> 70:             frame.setVisible(true);
> 71:         });

There must be `robot.waitForIdle` to ensure the UI becomes visible on the screen before you get the location of the combo box on the screen.

It's also common to add `robot.delay(1000)`.

test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 80:

> 78:         }
> 79:         catch (Exception e) {
> 80:             throw new RuntimeException(e);

Swing is not thread-safe, you should get the location and size on EDT too.
Suggestion:

        try {
            SwingUtilities.invokeAndWait(() ->{
                comboBoxLocation = comboBox.getLocationOnScreen();
                comboBoxSize = comboBox.getSize();
                
                closeButton.doClick();
            });
        }
        catch (Exception e) {
            throw new RuntimeException(e);

test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 80:

> 78:         }
> 79:         catch (Exception e) {
> 80:             throw new RuntimeException(e);

There's no need to catch the exception, just let it propagate.

test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 89:

> 87:     public static void clickComboBox() {
> 88:         int padding = 10;
> 89:         robot.mouseMove(comboBoxLocation.x + comboBoxSize.width - padding, comboBoxLocation.y + comboBoxSize.height / 2);

I'm sure this line doesn't fit into 80-column limit, I recommend wrapping it.
Suggestion:

        robot.mouseMove(comboBoxLocation.x + comboBoxSize.width - padding,
                        comboBoxLocation.y + comboBoxSize.height / 2);


Padding or offset could be a constant declared statically.

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

PR Review: https://git.openjdk.org/jdk/pull/24624#pullrequestreview-2764590444
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042302246
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042300846
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042306252
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042290016
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042292563
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042297780