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

Tue Jul 12 13:42:49 UTC 2016

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> 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>> 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