<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:10:39 UTC 2016
Hi Prasanta,
Kindly request you to review the webrev raised with inputs taken from reviewers in the mail trail. Thank you in advance.
With Regards,
Avik Niyogi
> On 15-Jul-2016, at 11:37 am, Avik Niyogi <avik.niyogi at oracle.com> wrote:
>
> I will ask Prasanta to review since it is not a native code change.
>
>> On 15-Jul-2016, at 11:35 am, Praveen Srivastava <praveen.s.srivastava at oracle.com <mailto:praveen.s.srivastava at oracle.com>> wrote:
>>
>> Rajeev is on sick leave, can someone else like Prasanta or Manajit review ?
>> See if you can push it today..
>>
>> Thanks
>> Praveen
>>
>>
>> From: Avik Niyogi
>> Sent: Friday, July 15, 2016 11:35 AM
>> To: Rajeev Chamyal
>> Cc: Praveen Srivastava; swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
>> Subject: Re: <Swing Dev> 8160438: [PIT][macosx] [TEST_BUG] javax/swing/plaf/nimbus/8057791/bug8057791.java fails
>>
>> 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 <mailto: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/2ceea042/attachment.html>
More information about the swing-dev
mailing list