On Mon, 15 Apr 2024 08:34:43 GMT, Jayathirth D V <[email protected]> wrote:

>> Abhishek Kumar has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Review comment update
>
> test/jdk/javax/swing/JComboBox/DisabledComboBoxFontTestAuto.java line 40:
> 
>> 38: import static java.awt.image.BufferedImage.TYPE_INT_ARGB;
>> 39: 
>> 40: /*
> 
> Usually we keep jtreg comment section before imports.

There was a discussion going on regarding placement of jtreg tags here 
https://corparch-core-srv.slack.com/archives/CNQ8SN9P1/p1707238824764849. We 
updated jtreg tags aftre imports section in recent test sprint.

So, kept it below imports.

> test/jdk/javax/swing/JComboBox/DisabledComboBoxFontTestAuto.java line 58:
> 
>> 56: 
>> 57:     private static void createCombo() {
>> 58:         combo = new JComboBox();
> 
> If we can use unicode blank character instead of "Simple JComboBox" text. I 
> think we make this test more robust and avoid tolerance checks.

Updated.

> We are saving these images even in the case where test passes. In my local 
> macOS run i see 16 images getting saved in JTwork folder. This will end up 
> taking good amount of space in our CI test systems.
We should save image only in case of test failure foe debugging purpose.

Updated to save the image only when test fails.

> Also i see images with Mac OS X***.png filename and all of them are just 
> blank and there is no combobox. Is that fine?

Even I also observed the same but not sure about this.

> test/jdk/javax/swing/JComboBox/DisabledComboBoxFontTestAuto.java line 125:
> 
>> 123:                 dColor2 = new Color(disabledImage2.getRGB(x + 1, y - 
>> 1));
>> 124:             } else if (lafName.equals("GTK+")) {
>> 125:                 // In GTK LAF, the ComboBox sizes are different and
> 
> Is there any way to get these internal offsets programmatically and not use 
> constants?

Removed the offsets as after having unicode blank character there is no need of 
these.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18644#discussion_r1566678112
PR Review Comment: https://git.openjdk.org/jdk/pull/18644#discussion_r1566680245
PR Review Comment: https://git.openjdk.org/jdk/pull/18644#discussion_r1566682142
PR Review Comment: https://git.openjdk.org/jdk/pull/18644#discussion_r1566680611

Reply via email to