<Swing Dev> 8160438: [PIT][macosx] [TEST_BUG] javax/swing/plaf/nimbus/8057791/bug8057791.java fails

Avik Niyogi avik.niyogi at oracle.com
Tue Jul 12 05:24:15 UTC 2016


Hi All,
A gentle reminder, please review my code changes.

With Regards,
Avik Niyogi

> On 08-Jul-2016, at 4:24 pm, Yuri Nesterenko <yuri.nesterenko at oracle.com> wrote:
> 
> It pass for me if I provide some time to process native events
> after the user activity simulation. For instance,
> setAutoDelay(50) for robot does that plus waitForIdle()
> after every mouseMove(). In this case, the part with that additional
> check and a (misleading, I think) comment about the color profiles
> may be removed. The test would take much more time though, and
>   @run main/timeout=600 bug8057791
> line would be necessary.
> 
> Thanks,
> -yan
> 
> On 07/08/2016 04:28 AM, Avik Niyogi wrote:
>> The test does not pass if mac specific check is not done for
>> backgroundcolor.
>> The check is required to get the same values from BufferedImage as they
>> are the same as found with Digital Color Meter.
>> This test case fixes that.
>> Please provide inputs if this fix will get a +1 or if not any positive
>> inputs to modify the test case will be welcome.
>> 
>> With Regards,
>> Avik Niyogi
>>> On 07-Jul-2016, at 9:51 pm, Semyon Sadetsky
>>> <semyon.sadetsky at oracle.com <mailto:semyon.sadetsky at oracle.com>> wrote:
>>> 
>>> 
>>> 
>>> On 7/7/2016 6:30 PM, Yuri Nesterenko wrote:
>>>> On 07/07/2016 06:15 PM, Semyon Sadetsky wrote:
>>>>> 
>>>>> 
>>>>> On 7/7/2016 5:58 PM, Yuri Nesterenko wrote:
>>>>>> On 07/07/2016 05:35 PM, Yuri Nesterenko wrote:
>>>>>>> On 07/07/2016 05:04 PM, Semyon Sadetsky wrote:
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On 07.07.2016 16:35, Avik Niyogi wrote:
>>>>>>>>> Hi Semyon,
>>>>>>>>> 
>>>>>>>>> Thank you for the review comment.
>>>>>>>>> 
>>>>>>>>> In Mac OS X, *System Preferences > Displays > Colors > Display
>>>>>>>>> Profile*  section, the default value is *Color LCD*.
>>>>>>>>> This causes a failure in some test cases which uses robot.The colour
>>>>>>>>> configuration it expects to use is the *Generic RGB Profile*.
>>>>>>>>> That is what “Non-generic display settings” means.
>>>>>>>> AFAIK there are instruction that tests should be executed using color
>>>>>>>> profile with no color corrections on OS X. I guess there are many
>>>>>>>> other
>>>>>>>> tests that fail with color correction.
>>>>>>>> If this is a root cause than the bug doesn't need to be fixed.
>>>>>>> 
>>>>>>> Well, I filed this bug and I used Generic RGB on all my
>>>>>>> test machines. There may be additional precautions in the tests
>>>>>>> about that but misconfiguration is not the root case here.
>>>>>>> That said, I feel vary about attempts to guess Apple colors
>>>>>>                   wary I mean
>>>>>>> in non-generic profiles.
>>>>> Yuri. Do you mean that the non-generic is not a root case?
>>>> No: I had Generic RGB everywhere, and it failed for me.
>>>> I should say that the new version of the test properly
>>>> fails with b120 and pass with current PIT. And colors
>>>> are visibly different in the two builds: so the test works OK now.
>>> That contradicts with the description of the fix.
>>> Probably the test does unnecessary care about the non-Generic profiles.
>>> 
>>> 159                 if (!foundMatch && isMac()) {
>>> 160                     //One more chance for Mac due to non-Generic
>>> display setting
>>> 161                     detectedColor = new Color(img.getRGB(x, y), true);
>>> 162                     if (detectedColor.equals(colorCheck)) {
>>> 163                         foundMatch = true;
>>> 164                         break checkLoops;
>>> 165                     }
>>> 166                 }
>>> 
>>> Does it mean that the code above, which is necessary to serve
>>> non-Generic profiles on OS X, may be removed and the test still passes?
>>> 
>>> --Semyon
>>>> 
>>>> -yan
>>>> 
>>>>> 
>>>>> --Semyon
>>>>>>> 
>>>>>>> -yan
>>>>>>> 
>>>>>>> 
>>>>>>>> 
>>>>>>>> --Semyon
>>>>>>>>> 
>>>>>>>>> Regarding “Negative scenarios”, these include cases where
>>>>>>>>> colours are
>>>>>>>>> different from the expected background or foreground colors
>>>>>>>>> is checked with the robot and BufferedImage respectively to
>>>>>>>>> *eliminate
>>>>>>>>> false positives due to coincidentally finding a match* by using
>>>>>>>>> robot
>>>>>>>>> or BufferedImage.
>>>>>>>>> 
>>>>>>>>> Please find my changes appropriating the inputs provided.
>>>>>>>>> I removed the variable foundMatch as I found that it is not required
>>>>>>>>> at all and incorporated the suggestion to use return instead of a
>>>>>>>>> labelled break.
>>>>>>>>> 
>>>>>>>>> http://cr.openjdk.java.net/~aniyogi/8160438/webrev.01/
>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.01/>
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>> On 07-Jul-2016, at 6:30 pm, Semyon Sadetsky
>>>>>>>>>> <semyon.sadetsky at oracle.com <mailto:semyon.sadetsky at oracle.com>>
>>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>> Hi Avik,
>>>>>>>>>> 
>>>>>>>>>> could you clarify what is "Non-generic display settings"? Is it
>>>>>>>>>> known
>>>>>>>>>> bug on OS X?
>>>>>>>>>> And also please be more specific on "negative scenarios" why
>>>>>>>>>> they are
>>>>>>>>>> necessary ?
>>>>>>>>>> 
>>>>>>>>>> Also could you replace labeled break with "return foundMatch; "
>>>>>>>>>> 
>>>>>>>>>> --Semyon
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> On 07.07.2016 15:11, Avik Niyogi wrote:
>>>>>>>>>>> Hi All,
>>>>>>>>>>> 
>>>>>>>>>>> Kindly review the fix for JDK9.
>>>>>>>>>>> 
>>>>>>>>>>> *Bug:
>>>>>>>>>>> *<https://bugs.openjdk.java.net/browse/JDK-8160438>https://bugs.openjdk.java.net/browse/JDK-8160438
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> *Webrev:
>>>>>>>>>>> *<http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.00/>http://cr.openjdk.java.net/~aniyogi/8160438/webrev.00/
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> *Issue: *test javax/swing/plaf/nimbus/8057791/bug8057791.java
>>>>>>>>>>> consistently fails on OS X 10.10
>>>>>>>>>>> 
>>>>>>>>>>> *Cause: * Due to bug in detecting color for Non-generic display
>>>>>>>>>>> settings for Mac hardware, test case failed
>>>>>>>>>>> 
>>>>>>>>>>> *Fix: *Positive and negative scenarios was added to check the
>>>>>>>>>>> colour
>>>>>>>>>>> for the Nimbus LAF foreground and background colours.
>>>>>>>>>>> 
>>>>>>>>>>> With Regards,
>>>>>>>>>>> Avik Niyogi
>> 
> 




More information about the swing-dev mailing list