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