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.niy...@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.srivast...@oracle.com <mailto:praveen.s.srivast...@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@openjdk.java.net >> <mailto:swing-dev@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.scherba...@oracle.com <mailto:alexandr.scherba...@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.scherba...@oracle.com <mailto:alexandr.scherba...@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.nestere...@oracle.com >> <mailto:yuri.nestere...@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.sadet...@oracle.com <mailto:semyon.sadet...@oracle.com> >> <mailto:semyon.sadet...@oracle.com <mailto:semyon.sadet...@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.sadet...@oracle.com <mailto:semyon.sadet...@oracle.com> >> <mailto:semyon.sadet...@oracle.com> <mailto:semyon.sadet...@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
