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