RFR: 8341162: Open source some of the AWT window test [v2]

[Alexey Ivanov]

On Thu, 3 Oct 2024 07:45:21 GMT, Jayathirth D V <jdv at openjdk.org> wrote:

>> Clean up and open source some of the AWT window tests.
>> Automated test `OwnedWindowShowTest.java` is verified in our CI also and it passes on all platforms.
>
> Jayathirth D V has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Remove not needed setVisible calls
>  - Update based on review comments

test/jdk/java/awt/Window/LocationByPlatform/TestLocationByPlatform.java line 48:

> 46:             intersecting other Frames or stacked over normal frame with some
> 47:             offset. Another has its location explicitly set to (0, 450).
> 48:             Please verify that the frames are situated correctly.

Suggestion:

            Please verify that the frames are located correctly on the screen.

test/jdk/java/awt/Window/LocationByPlatform/TestLocationByPlatform.java line 54:

> 52:             surrounding the client area of frame with no pixels between it and
> 53:             the frame's decorations. Press Pass if this all is true,
> 54:             otherwise press Fail.

Suggestion:


            Please verify that the picture inside of frames looks the same
            and consists of red descending triangle occupying exactly the bottom
            half of the frame. Also, verify that there is a blue rectangle exactly
            surrounding the client area of frame with no pixels between it and
            the frame's decorations. Press Pass if this all is true,
            otherwise press Fail.


Adding a blank line adds structure to the instructions and makes them easier to read.

test/jdk/java/awt/Window/LocationByPlatform/TestLocationByPlatform.java line 63:

> 61:             .columns(40)
> 62:             .build();
> 63:         EventQueue.invokeAndWait(() -> createUI());

Suggestion:

        EventQueue.invokeAndWait(TestLocationByPlatform::createUI);

I prefer using method references; I don't insist on it though.

test/jdk/java/awt/Window/LocationByPlatform/TestLocationByPlatform.java line 86:

> 84:                 g.drawLine(399, 0, 399, 399);
> 85:             }
> 86:         };

I suggest creating a custom class that extends `Canvas`. The class will encapsulate all this logic.

You'll create an instance of this class and put it to each frame without duplicating all the code to render the red triangle and blue rectangle.

test/jdk/java/awt/Window/OwnedWindowShowTest/OwnedWindowShowTest.java line 38:

> 36: 
> 37: public class OwnedWindowShowTest {
> 38:     public static void main(String args[]) throws Exception {

Use Java-style array declaration:
Suggestion:

    public static void main(String[] args) throws Exception {

test/jdk/java/awt/Window/OwnedWindowShowTest/OwnedWindowShowTest.java line 39:

> 37: public class OwnedWindowShowTest {
> 38:     public static void main(String args[]) throws Exception {
> 39:         EventQueue.invokeAndWait(() -> runTest());

Suggestion:

        EventQueue.invokeAndWait(OwnedWindowShowTest::runTest);

test/jdk/java/awt/Window/OwnedWindowShowTest/OwnedWindowShowTest.java line 51:

> 49:             if (parent != null) {
> 50:                 parent.dispose();
> 51:             }

The condition `parent != null` is always `true` when `if`-statement is reached.
Suggestion:

            parent.dispose();


You may also want to dispose of `window` and `owner` too.

test/jdk/java/awt/Window/ShowWindowTest/ShowWindowTest.java line 52:

> 50:             2. Click on the "Show" button. A Window with a "Hello World" Label
> 51:             should appear
> 52:             3. If a Window does not appear, the test failed.

Suggestion:

            2. Click on the "Show" button. A window with a "Hello World" Label
            should appear
            3. If a Window does not appear, the test fails.

test/jdk/java/awt/Window/ShowWindowTest/ShowWindowTest.java line 71:

> 69:         frame.add(hideButton = new Button("Hide"));
> 70:         showButton.addActionListener(new ShowWindowTest());
> 71:         hideButton.addActionListener(new ShowWindowTest());

Suggestion:

        frame.add(hideButton = new Button("Hide"));
        
        ActionListener handler = new ShowWindowTest();
        showButton.addActionListener(handler);
        hideButton.addActionListener(handler);

You can use the same instance for both.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786555094
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786556979
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786562729
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786560391
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786565422
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786564437
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786572634
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786582042
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1786585906