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

Damon Nguyen dnguyen at openjdk.org
Thu Mar 21 16:51:36 UTC 2024


On Thu, 21 Mar 2024 14:14:17 GMT, Alexander Zvegintsev <azvegint 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 33:
> 
>> 31:  *         on HiDPI + non-HiDPI display pair.
>> 32:  * @run main MultiDisplayTest
>> 33:  */
> 
> We usually move this block just before a class declaration, for better readability.

Moved.

> test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 58:
> 
>> 56:             """
>> 57:              This test is for OS X or Windows only.
>> 58:              For other OSes please simply push "Pass".
> 
> `please simply push "Pass"` usually means that it can be replaced with `PassFailJFrame.forcePass()`, but in this case the `@requires (os.family...` can be used to avoid running the test on unwanted platforms.

Added jtreg `@requires` tag.

> test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 93:
> 
>> 91:         generateImage(1, Color.BLACK), generateImage(2, Color.BLUE)});
>> 92: 
>> 93:     public static void main(String[] args) throws Exception {
> 
> While the `@requires (os.family...` should be enough, we can add something like
> 
> if (!checkOS()) {
>     throw new SkippedException("..."); //jtreg.SkippedException
> }

Added. Thanks. Thought about not needing this at all, but it doesn't hurt to add.

> test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 109:
> 
>> 107:         frame.setLayout(new BorderLayout());
>> 108:         Button b = new Button("Start");
>> 109:         b.setEnabled(checkOS());
> 
> With the changes mentioned above, this will become obsolete.

Removed the `checkOS()` call.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534277257
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534277092
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534276882
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534277948


More information about the client-libs-dev mailing list