<AWT Dev> Review request for 8145116: [TEST_BUG] Incorrect binary comparison in java/awt/event/KeyEvent/ExtendedModifiersTest/ExtendedModifiersTest.java

[Semyon Sadetsky]

Hi Ambarish,

Looks good to me.

Just for future, try to avoid  reformatting of the code that remains 
unchanged in the fix.

--Semyon

On 12/15/2015 3:33 PM, Ambarish Rapte wrote:
>
> Hi Jay,
>
> Thanks for the review comments,
>
> Please check the updated webrev as per the review comments,
>
> http://cr.openjdk.java.net/~arapte/8145116/webrev.01/ 
> <http://cr.openjdk.java.net/%7Earapte/8145116/webrev.01/>
>
> Changes:
>
> 1.Added bug id
>
> 2.Changed variable name LOCK to lock
>
> Thanks,
>
> Ambarish
>
> *From:*Jayathirth D V
> *Sent:* Tuesday, December 15, 2015 2:12 PM
> *To:* Ambarish Rapte
> *Cc:* awt-dev at openjdk.java.net
> *Subject:* RE: <AWT Dev> Review request for 8145116: [TEST_BUG] 
> Incorrect binary comparison in 
> java/awt/event/KeyEvent/ExtendedModifiersTest/ExtendedModifiersTest.java
>
> Hi,
>
> Test case should validate based on expected values and not on value 
> which we are getting from ExtendedModifiers. So we should pass 
> expected value to assertEQ(). Changes are fine related to it.
>
> But according to coding guidelines variables which contain constants 
> should be declared in capital letters. Change the “LOCK” variable 
> which is declared in capital letters.
>
> Also add new Bug ID in jtreg description.
>
> Thanks,
>
> Jay
>
> *From:* Ambarish Rapte
> *Sent:* Thursday, December 10, 2015 9:01 PM
> *To:* Semyon Sadetsky; Prasanta Sadhukhan; Rajeev Chamyal; 
> awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
> *Subject:* <AWT Dev> Review request for 8145116: [TEST_BUG] Incorrect 
> binary comparison in 
> java/awt/event/KeyEvent/ExtendedModifiersTest/ExtendedModifiersTest.java
>
> Hi,
>
>                 Please review the fix patch for JDK9,
>
>                 Bug: https://bugs.openjdk.java.net/browse/JDK-8145116
>
>                 Webrev: 
> http://cr.openjdk.java.net/~arapte/8145116/webrev.00/ 
> <http://cr.openjdk.java.net/%7Earapte/8145116/webrev.00/>
>
> Issue:
>
> This is a test code issue.
>
> The binary comparison for validating the result was incorrect.
>
> The test would PASS even for wrong inputs.
>
> Please refer to the bug description for how to provide wrong inputs.
>
> Fix:
>
> Fixed the binary comparison.
>
> Please compare line number 177 from left with line 185 on right.
>
> Remaining changes are related to coding guidelines, robot synchronization.
>
> Thanks,
>
> Ambarish
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20151215/af323b60/attachment.html>