RFR: 8280913: Create a regression test for JRootPane.setDefaultButton() method [v4]

[Alexey Ivanov]

On Thu, 3 Feb 2022 11:20:50 GMT, Manukumar V S <mvs at openjdk.org> wrote:

>> This tests the behaviour of the method JRootPane.setDefaultButton() in different platforms with different LAFs.
>> As per the spec https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/javax/swing/JRootPane.html#setDefaultButton(javax.swing.JButton), "The default button is the button which will be activated when a UI-defined activation event (typically the Enter key) occurs in the root pane regardless of whether or not the button has keyboard focus (unless there is another component within the root pane which consumes the activation event, such as a JTextPane). ".
>> 
>> This test is run multiple times in different platforms and got 100% success in mac and windows, linux.
>
> Manukumar V S has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8280913: Create a regression test for JRootPane.setDefaultButton() method

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/JRootPane/DefaultButtonTest.java line 39:

> 37:  * @bug 8280913
> 38:  * @summary Check whether the default button is honored when <Enter> key is
> 39:  * pressed while the focus is on the frame.

…“while the focus is on the frame” is not quite correct. The default button should get pressed when focus is on whatever component that doesn't consume Enter. I suggest dropping this part from the description.

test/jdk/javax/swing/JRootPane/DefaultButtonTest.java line 44:

> 42: public class DefaultButtonTest {
> 43:     volatile boolean buttonPressed = false;
> 44:     JFrame frame;

I'd add `private` to the declaration.

Assigning `false` is redundant. It's the default value and you explicitly assign `false` to it in each L&F iteration.

test/jdk/javax/swing/JRootPane/DefaultButtonTest.java line 52:

> 50:         } finally {
> 51:             test.disposeFrame();
> 52:         }

This partly duplicates the code in `runTest`.
I'd suggest putting try-finally inside the for-loop in `runTest`.

test/jdk/javax/swing/JRootPane/DefaultButtonTest.java line 78:

> 76:         JPanel panel = new JPanel();
> 77:         panel.setLayout(new FlowLayout());
> 78:         JButton button1 = new JButton("button1");

May I suggest adding:

panel.add(new JTextField("Text field"));

The text field will get the focus on the frame. Pressing Enter should still activate the default button. I think it's a better test compared to explicitly requesting the focus to the root pane because it's more natural.

If you do this, you can drop the line:

frame.getRootPane().requestFocus();

test/jdk/javax/swing/JRootPane/DefaultButtonTest.java line 89:

> 87:         frame.add(panel);
> 88: 
> 89:         frame.setSize(200, 200);

Can be replaced with

frame.pack();

test/jdk/javax/swing/JRootPane/DefaultButtonTest.java line 96:

> 94: 
> 95:     public void runTest() throws Exception {
> 96:         Robot robot = new Robot();

Add `robot.setAutoDelay(100);` then you can drop `robot.delay(100);` between key press and release.

test/jdk/javax/swing/JRootPane/DefaultButtonTest.java line 117:

> 115:                 throw new RuntimeException("Test Failed, button not pressed for L&F: " + lafName);
> 116:             }
> 117:             disposeFrame();

The frame must be disposed of on EDT. So either

SwingUtilities.invokeAndWait(this::disposeFrame);

or wrap the body of `disposeFrame` method into `SwingUtilities.invokeAndWait`.

test/jdk/javax/swing/JRootPane/DefaultButtonTest.java line 121:

> 119:     }
> 120: 
> 121: }

Please add line break at the end of the file.

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

PR: https://git.openjdk.java.net/jdk/pull/7278