RFR: 8351884: Refactor bug8033699.java test code [v3]

Alexey Ivanov aivanov at openjdk.org
Fri Apr 25 19:46:50 UTC 2025


On Fri, 25 Apr 2025 16:55:40 GMT, Rajat Mahajan <rmahajan at openjdk.org> wrote:

>> Details:
>> Refactored code as requested in the Bug description.
>> 
>> Tested and verified the test passes.
>
> Rajat Mahajan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   code changes as per code review

Looks good to me, except for minor nits.

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 62:

> 60:     public static void main(String[] args) throws Throwable {
> 61:         robot = new Robot();
> 62:         SwingUtilities.invokeAndWait(() -> {

I wonder why you removed the blank line, I'm strongly for restoring it here.

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 64:

> 62:         SwingUtilities.invokeAndWait(() -> {
> 63:             focusManager = KeyboardFocusManager.getCurrentKeyboardFocusManager();
> 64:         });

Suggestion:

        SwingUtilities.invokeAndWait(() ->
                focusManager = KeyboardFocusManager.getCurrentKeyboardFocusManager());

Braces are redundant for one statement (in lambdas), or _“Statement lambda can be replaced with expression lambda.”_

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 66:

> 64:         });
> 65:         // Get all installed Look and Feels
> 66:         UIManager.LookAndFeelInfo[] lafs = UIManager.getInstalledLookAndFeels();

Suggestion:

            focusManager = KeyboardFocusManager.getCurrentKeyboardFocusManager();
        });

        UIManager.LookAndFeelInfo[] lafs = UIManager.getInstalledLookAndFeels();

I'd add a blank here, too, and remove the comment since `getInstalledLookAndFeels` is already self-documenting.

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 170:

> 168: 
> 169:         // Create main vertical box
> 170:         Box mainBox = Box.createVerticalBox();

Suggestion:

        Box mainBox = Box.createVerticalBox();

The comment is redundant, it doesn't add anything to what one could infer from the statement.

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 176:

> 174:         mainBox.add(btnEnd);
> 175: 
> 176:         mainFrame.getContentPane().add(mainBox);

Suggestion:

        mainFrame.add(mainBox);

At this point, @honkar-jdk's can be applied. I'll leave it to your discretion, either way is fine with me.

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

Marked as reviewed by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24384#pullrequestreview-2795116203
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060786756
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060798663
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060790607
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060792449
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060794078


More information about the client-libs-dev mailing list