RFR: 8328368: Convert java/awt/image/multiresolution/MultiDisplayTest/MultiDisplayTest.java applet test to main [v2]

[Alexey Ivanov]

On Thu, 21 Mar 2024 16:51:36 GMT, Damon Nguyen <dnguyen at openjdk.org> wrote:

>> Convert java/awt/image/multiresolution/MultiDisplayTest/MultiDisplayTest.java applet test to main using PassFailJFrame
>
> Damon Nguyen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 62:

> 60:              - 2nd display is non-HiDPI.
> 61: 
> 62:              In other cases please simply push "Pass".

These conditions can be detected. On the other hand, since it's a manual test, the tester could enable or connect a second monitor, so it makes sense to preserve the instructions.

test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 68:

> 66:              Then drag parent / child to different displays and check
> 67:              that the proper image is shown for every window
> 68:              (must be "black 1x" for non-HiDPI and "blue 2x" for HiDPI).

What if the scale is not 200%? It's possible on Windows and it's not uncommon.

test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 79:

> 77:              and Mission Control behavior.
> 78: 
> 79:              Close the windows.

Won't it fail the test?

test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 84:

> 82:             """;
> 83: 
> 84:     private static final int W = 200, H = 200;

Suggestion:

    private static final int W = 200;
    private static final int H = 200;

Java Coding Guidelines do not recommend having several declarations on the same line.

test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 141:

> 139:         public ParentFrame() {
> 140:             EventQueue.invokeLater(this::CreateUI);
> 141:         }

This looks weird…

As far as I can see, the frame is created when Start button is clicked, which implies this operation is running on EDT. There's no reason to use `invokeLater`, all the code from `CreateUI` method could be placed into the constructor.

test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 165:

> 163:             super(f);
> 164:             EventQueue.invokeLater(this::CreateUI);
> 165:         }

The same comment, `invokeLater` seems redundant.

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

PR Review: https://git.openjdk.org/jdk/pull/18354#pullrequestreview-1952880721
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534322342
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534330435
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534327277
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534328684
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534338034
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534338996