<Swing Dev> 8160438: [PIT][macosx] [TEST_BUG] javax/swing/plaf/nimbus/8057791/bug8057791.java fails
Alexandr Scherbatiy
alexandr.scherbatiy at oracle.com
Thu Jul 14 15:16:25 UTC 2016
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>> 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/>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160714/117d93e8/attachment.html>
More information about the swing-dev
mailing list