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

Avik Niyogi avik.niyogi at oracle.com
Fri Jul 15 06:04:45 UTC 2016 

Another gentle reminder, please review my code changes. Thank you in advance.
With Regards,
Avik Niyogi
> On 14-Jul-2016, at 8:46 pm, Alexandr Scherbatiy <alexandr.scherbatiy at oracle.com> wrote:
> 
> 
> The fix looks good to me.
> 
> Thanks,
> Alexandr.
> 
> On 7/14/2016 9:53 AM, Avik Niyogi wrote:
>> Hi All,
>> Please find my webrev below for review which includes changes as perceived from inputs provided.
>> 
>> http://cr.openjdk.java.net/~aniyogi/8160438/webrev.02/ <http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.02/>
>> 
>> With Regards,
>> Avik Niyogi
>>> On 12-Jul-2016, at 7:12 pm, Alexandr Scherbatiy <alexandr.scherbatiy at oracle.com <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>> 
>>> On 7/12/2016 8:24 AM, Avik Niyogi wrote:
>>>> 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 <mailto: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.
>>>    Some more comments to the previous ones:
>>>  - runColorTestCase() uses JList and should be run on EDT
>>>  - "if (tryNimbusLookAndFeel()) {" It is supposed that the Nimbus L&F is supported on all platforms. May be it is better to fail the test if the Nimbus L&F is not set.
>>>  - "if (osName.contains("Mac")) { isMac = true; }" can be simplified to "return osName.contains("Mac")"
>>>  - " if (!"".equals(errorString)) {" can be simplified to !errorString.isEmpty()
>>> 
>>>  Thanks,
>>>  Alexandr.
>>>>> 
>>>>> 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> <mailto: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/>
>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/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> <mailto: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>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/%7Eaniyogi/8160438/webrev.00/>http://cr.openjdk.java.net/~aniyogi/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
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160715/8825d636/attachment.html>

More information about the swing-dev mailing list