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

Alexey Ivanov aivanov at openjdk.java.net
Wed Feb 2 16:22:06 UTC 2022


On Sun, 30 Jan 2022 06:30:43 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 25:

> 23: 
> 24: import javax.swing.*;
> 25: import java.awt.*;

Could you please expand the wildcard imports?
For JDK, `java.*` packages are usually above `javax.*`.

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

> 38:  */
> 39: public class DefaultButtonTest {
> 40:     static JFrame frame = new JFrame();

`JFrame` is part of Swing and must not be created or accessed from any thread but EDT.

I suggest creating the frame in `createUI` method along with other UI components. In this case, you need to dispose of the frame for each LaF.

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

> 103:             System.err.print("Error creating robot");
> 104:             e.printStackTrace();
> 105:             System.exit(1);

You must never call `System.exit` from a jtreg test: just throw the exception.

So in this case, you don't need try-catch here, just let `AWTException` propagate to main.

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

> 118:             } catch (Exception e) {
> 119:                 e.printStackTrace();
> 120:             }

Redundant, propagate the exception.

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

> 125:             robot.waitForIdle();
> 126: 
> 127:             if (buttonPressed) {

You can negate the condition and throw `RuntimeException`  to fail the test. And `testFailed` variable can be dropped.

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

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



More information about the client-libs-dev mailing list