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

[Jayathirth D V]

Updated changes are fine.

 

From: Ambarish Rapte 
Sent: Tuesday, December 15, 2015 6:03 PM
To: Jayathirth D V
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 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/

 

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: HYPERLINK "mailto:awt-dev at openjdk.java.net"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; HYPERLINK "mailto:awt-dev at openjdk.java.net"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/

 

 

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/6f9c080b/attachment-0001.html>