On Fri, 12 May 2023 12:26:52 GMT, Renjith Kannath Pariyangad <d...@openjdk.org> wrote:
>> Hi Reviewers, >> Noticed this test case not verifying all the cases which is intended. >> Modified it for improving the coverage for 3 type of popups (menu, context >> menu and combobox). >> >> Evaluating conditions: >> 1. Not enough space for showing popup downwards(default layout), it should >> show upwards >> 2. Window starts from negative position, Popup should show on visible area >> >> For achieving this following areas are modified >> - Updated isPopupOnScreen by adding Additional checks (like the position of >> combobox popup is always verified) >> - Menu creation made as function and reused for all menu creation. >> - Updated ComboPopupCheckListener class and modified its >> popupMenuWillBecomeInvisible function. Made it as generic and it is capable >> to evaluate any combo box's position if this class set as its listener. >> - This test case is not intended for multi monitor setup so added a check >> for identifying monitor, else error out. >> - Updated CTRL_MASK to CTRL_DOWN_MASK for removing depreciation warnings and >> removed some other warnings. >> >> Please review this >> >> Regards, >> Renjith. > > Renjith Kannath Pariyangad has updated the pull request incrementally with > one additional commit since the last revision: > > Updated test description, disabling all system shortcut is required to run > the test case on MAC Changes requested by aivanov (Reviewer). test/jdk/javax/swing/Popup/TaskbarPositionTest.java line 64: > 62: * @summary Tests the location of the heavy weight popup portion of > JComboBox, > 63: * JMenu and JPopupMenu. > 64: * On MAC disable all system shortcuts before executing this test case. Well, the test uses only one particular shortcut which was introduced in macOS Ventura. I'd rather have a more specific statement, something like this: The test uses Ctrl+Down Arrow (↓) which is a system shortcut on macOS, disable it in system settings, otherwise the test will fail. The systems which are exclusively used for running tests should have all system shortcuts disabled. It may be *unfeasible* to disable all the shortcuts on a system that you use regularly for doing your work as well as for running tests (while you're updating it or fixing a bug). ------------- PR Review: https://git.openjdk.org/jdk/pull/13578#pullrequestreview-1424983532 PR Review Comment: https://git.openjdk.org/jdk/pull/13578#discussion_r1192683506
