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

Jayathirth D V jdv at openjdk.org
Fri Oct 4 10:42:24 UTC 2024


On Fri, 4 Oct 2024 09:28:15 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

>> 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.

Updated.

> 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

Updated.

> 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"));

This seems intentional. Leaving as it is.

> 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.

Updated.

> 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.

Updated.

> 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.

Updated.

> 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.

Updated.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787524466
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787524723
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787525091
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787525562
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787525352
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787525774
PR Review Comment: https://git.openjdk.org/jdk/pull/21348#discussion_r1787525911


More information about the client-libs-dev mailing list