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

Damon Nguyen dnguyen at openjdk.org
Thu Mar 21 17:43:46 UTC 2024


On Thu, 21 Mar 2024 17:17:10 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Damon Nguyen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comments
>
> 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?

This was a part of the original instructions. I modified it to clarify that we need to close the Parent and Child windows, not the PassFailJFrame as well. Thanks

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

Fixed. Thanks

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

Agreed. I modified it as you suggested. Likewise for the child frame.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534351092
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534351797
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534351579


More information about the client-libs-dev mailing list