RFR: 8289547 : Update javax/swing/Popup/TaskbarPositionTest.java [v2]
Alexey Ivanov
aivanov at openjdk.org
Fri May 12 18:35:51 UTC 2023
On Fri, 12 May 2023 12:26:52 GMT, Renjith Kannath Pariyangad <duke at 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
More information about the client-libs-dev
mailing list