RFR: 8340015: Open source several AWT focus tests - series 7 [v2]

Harshitha Onkar honkar at openjdk.org
Tue Sep 17 23:43:07 UTC 2024


On Tue, 17 Sep 2024 04:06:52 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> Opensource few AWT focus tests
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   MToolkit removed

LGTM other than the minor inline review comments

test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 35:

> 33: import java.awt.Frame;
> 34: import java.awt.Panel;
> 35: import java.awt.Toolkit;

Unused import

test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 59:

> 57:                 .instructions(INSTRUCTIONS)
> 58:                 .rows((int) INSTRUCTIONS.lines().count() + 5)
> 59:                 .columns(35)

Looks better formatted with the following:

Suggestion:

                .rows((int) INSTRUCTIONS.lines().count() + 1)
                .columns(40)

test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 66:

> 64: 
> 65:     private static List<Window> createTestUI() {
> 66:         Panel panel = new Panel();

Unused variable can be removed.

test/jdk/java/awt/Focus/WindowDisposeFocusTest.java line 68:

> 66: 
> 67:     static public Window test(String[] args) {
> 68:         final JFrame frame = new JFrame();

Frame title is missing. 
Does setting a frame size look better here although the test is more to do with the dialogs?

test/jdk/java/awt/Focus/bug6435715.java line 78:

> 76:                 synchronized (this) {
> 77:                     try {
> 78:                         wait(3000);

Is large wait time required here? I think it can be reduced.

test/jdk/java/awt/Focus/bug6435715.java line 90:

> 88:         return fr;
> 89:     }
> 90: 

extra new line can be removed.

test/jdk/java/awt/Focus/bug6435715.java line 92:

> 90: 
> 91: }
> 92: 

extra new line can be removed.

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

Marked as reviewed by honkar (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21008#pullrequestreview-2311274622
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764150936
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764158164
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764151437
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764211515
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764216160
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764215060
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764215334


More information about the client-libs-dev mailing list