Re: 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@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@openjdk.java.net <mailto:awt-dev@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