RFR: 8341982: Simplify JButton/bug4323121.java

Abhishek Kumar abhiscxk at openjdk.org
Mon Oct 14 07:18:18 UTC 2024


On Fri, 11 Oct 2024 18:20:52 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

> The test `javax/swing/JButton/bug4323121.java` contains lots of unused methods.
> 
> I removed all the unused methods by extending `MouseAdapter`.
> 
> I use `CountDownLatch` to synchronise actions in the test.
> 
> The test still verifies `button.getModel().isArmed()` doesn't always return `true` for classes which extend `JButton`. I verified the updated test fails in 1.3.0 and passes in 1.4.0, so the test still reproduces the original problem.

test/jdk/javax/swing/JButton/bug4323121.java line 58:

> 56:     private static final CountDownLatch mouseEntered = new CountDownLatch(1);
> 57: 
> 58:     // Thread-safe by using the mouseEntered latch

Comment may be put before `private static final CountDownLatch mouseEntered = new CountDownLatch(1);`

test/jdk/javax/swing/JButton/bug4323121.java line 69:

> 67:             SwingUtilities.invokeAndWait(() -> {
> 68:                 button = new TestButton("gotcha");
> 69:                 button.addMouseMotionListener(eventHandler);

Don't think it is required anymore to add `MouseMotionListener`.

test/jdk/javax/swing/JButton/bug4323121.java line 70:

> 68:                 button = new TestButton("gotcha");
> 69:                 button.addMouseMotionListener(eventHandler);
> 70:                 button.addMouseListener(eventHandler);

You may get rid of `eventHandler` object (provided MouseMotionListener is not required) by adding the mouse listener similar to WindowFocusListener where you used WindowAdapter class and override WindowGainedFocus method. 
 
            button.addMouseListener(new MouseAdapter() {
                    @Override
                    public void mouseEntered(MouseEvent e) {
                        if (button.getModel().isArmed()) {
                            modelArmed = true;
                        }
                        mouseEntered.countDown();
                    }
                });

test/jdk/javax/swing/JButton/bug4323121.java line 88:

> 86:             });
> 87: 
> 88:             if (!windowGainedFocus.await(1, SECONDS)) {

Should we add a small delay before checking?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21475#discussion_r1798832613
PR Review Comment: https://git.openjdk.org/jdk/pull/21475#discussion_r1798833493
PR Review Comment: https://git.openjdk.org/jdk/pull/21475#discussion_r1798876961
PR Review Comment: https://git.openjdk.org/jdk/pull/21475#discussion_r1798877741


More information about the client-libs-dev mailing list