On Mon, 10 Jul 2023 20:12:25 GMT, Alexey Ivanov <[email protected]> wrote:
>> Harshitha Onkar has updated the pull request incrementally with one >> additional commit since the last revision: >> >> minor test changes > > src/java.desktop/macosx/native/libawt_lwawt/awt/CRobot.m line 322: > >> 320: >> 321: CGEventFlags flags = >> CGEventSourceFlagsState(kCGEventSourceStateHIDSystemState); >> 322: flags = (initFlags & allModifiersMask) | (flags & >> (!allModifiersMask)); > > Should `!allModifiersMask` be `~allModifiersMask`? That is _bitwise_ NOT > instead of _logical_ NOT. > > Suggestion: > > flags = (initFlags & allModifiersMask) | (flags & > (!allModifiersMask)); @aivanov-jdk That is correct, Thanks for catching it. It should be `~`. > test/jdk/java/awt/Robot/RobotModifierMaskTest.java line 261: > >> 259: boolean condition = >> expectedResult.equals(EXPECTED_RESULT_CTRL) >> 260: ? (jTextArea.getCaretPosition() >> 261: != >> Integer.parseInt(EXPECTED_RESULT_CTRL)) > > You pass different expected result strings but here you always compare > `expectedResult` to `EXPECTED_RESULT_CTRL`. Is it intentional? Do I miss > anything? > > I haven't run the test. The assertion for CTRL key is different for the rest and involves caret position comparison instead of expected strings (as in other cases), hence I have used `EXPECTED_RESULT_CTRL` to differentiate the cases. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14744#discussion_r1258918909 PR Review Comment: https://git.openjdk.org/jdk/pull/14744#discussion_r1258918223
