On Fri, 25 Feb 2022 18:55:24 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> DamonGuy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Changed robot autodelay time and added wait after creating gui. Added try 
>> finally block for frame disposal.
>
> test/jdk/javax/swing/JButton/HtmlButtonImageTest/HtmlButtonImageTest.java 
> line 67:
> 
>> 65:             } catch (ClassNotFoundException | InstantiationException | 
>> IllegalAccessException e) {
>> 66:                 throw new RuntimeException("Unsupported LookAndFeel: " + 
>> e);
>> 67:             }
> 
> You can let these exceptions propagate up and be thrown from `main` directly.
> 
> It was discussed in the comments that this bug is applicable to all 
> Look-and-Feels. But the test is macOS- and Aqua-specific.

Noted. I will propagate the exceptions to main with my next commit, along with 
the other changes you suggested.

The bug was initially made for MacOS only, but was mentioned to be an issue on 
after Windows as well. I am able to reproduce the issue on other LAFs as well, 
but it seems that the root issue is different in other LAFs. I was advised to 
focus the fix on the Mac specific fix for now, so I made my changes to Aqua 
button and set the test to macOS.

> test/jdk/javax/swing/JButton/HtmlButtonImageTest/HtmlButtonImageTest.java 
> line 141:
> 
>> 139:             System.out.println("-- Passed");
>> 140:         }
>> 141:     }
> 
> I suggest using variable arguments:
> Suggestion:
> 
>     private static void testImageCentering(Color... colors) {
>         // check if all colors at each edge of square are red
>         for (Color c : colors) {
>             if (!checkRedness(c)) {
>                 throw new RuntimeException("HTML image not centered in 
> button");
>             }
>         }
>        System.out.println("-- Passed");
>     }
> 
> The call to this function remains the same. And you can easily add additional 
> points to be checked if needed.

Awesome idea, added.

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

PR: https://git.openjdk.java.net/jdk/pull/7310

Reply via email to