RFR: 8341298: Open source more AWT window tests [v2]

Abhishek Kumar abhiscxk at openjdk.org
Fri Oct 4 09:47:37 UTC 2024


On Fri, 4 Oct 2024 08:09:20 GMT, Jayathirth D V <jdv at openjdk.org> wrote:

>> Clean up and open source more of the AWT window tests.
>> Automated tests `PopupProxyCrash` and `NoResizeEvent` are also verified in our CI.
>
> Jayathirth D V has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add headful tag in PopupProxyCrash Test

test/jdk/java/awt/Window/LocationByPlatformWithControls/TestLocationByPlatformWithControls.java line 50:

> 48:     Checkbox undecoratedCB, defaultLocationCB, visibleCB,
> 49:         iconifiedCB, maximizedCB;
> 50:     Button createB, packB, moveB, resizeB, reshapeB, disposeB;

It is recommended to declare variables on separate lines.

test/jdk/java/awt/Window/LocationByPlatformWithControls/TestLocationByPlatformWithControls.java line 54:

> 52:     public static void main(String[] args) throws Exception {
> 53:         String INSTRUCTIONS = """
> 54:             This test is to check that LocationByPlatform works with  other

Suggestion:

            This test is to check that LocationByPlatform works with other

test/jdk/java/awt/Window/LocationByPlatformWithControls/TestLocationByPlatformWithControls.java line 158:

> 156:             panel.add(new Button ("Test Button 1"));
> 157:             panel.add(new Button ("Test Button  2"));
> 158:             panel.add(new Button ("Test Button   3"));

Suggestion:

            panel.add(new Button ("Test Button 2"));
            panel.add(new Button ("Test Button 3"));

test/jdk/java/awt/Window/ProxyCrash/PopupProxyCrash.java line 41:

> 39: import java.awt.BorderLayout;
> 40: import java.awt.Button;
> 41: import java.awt.Color;

Awt imports before swing.

test/jdk/java/awt/Window/ProxyCrash/PopupProxyCrash.java line 125:

> 123:         frame.add(parent, BorderLayout.CENTER);
> 124:         frame.setSize(200,200);
> 125:         frame.pack();

invoking both setSize and pack is redundant.

test/jdk/java/awt/Window/ProxyCrash/PopupProxyCrash.java line 167:

> 165:             robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);
> 166:             robot.delay(500);
> 167:             robot.waitForIdle();

Seen many review comments where it is advised to invoke delay after waitForIdle.

test/jdk/java/awt/Window/WindowToFrontTest/WindowToFrontTest.java line 62:

> 60:         EventQueue.invokeAndWait(() -> createUI());
> 61:         passFailJFrame.awaitAndCheck();
> 62:     }

Please add a blank line after this.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787438638
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787439066
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787441596
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787446508
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787452059
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787457348
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787459838


More information about the client-libs-dev mailing list