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

[Jayathirth D V]

On Thu, 3 Oct 2024 17:20:59 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> 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/OwnedWindowShowTest/OwnedWindowShowTest.java line 47:
> 
>> 45:             Window owner = new Window(parent);
>> 46:             Window window = new Window(owner);
>> 47:             window.setVisible(true);
> 
> You may add a comment here that `setVisible(true)` should not throw any exception.
> 
> Yes, it is stated in the test summary, yet repeating it here would make it clearer.

Updated.

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

Removed if check. If parent is disposed, i expect child components also to be disposed.
I don't see any UI components after test is finished. Do we need to explicitly dispose all child components?

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

Updated and added more clarity.

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

Updated.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787236850
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787236083
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787236702
PR Review Comment: https://git.openjdk.org/jdk/pull/21284#discussion_r1787236996