Avoid false positives when testing Files app cr-menu state and commands |
|||
Issue descriptionLooking over the main.html cr-menus html, there appears to be a general problem now the app has so many menus with the same command id names, "paste", "copy", eg ... As noticed in issue 871771 , queries for the context menu item states could be made more precise, eg., by prefixing them with #file-context-menu:not([hidden]), since they can currently select state from a hidden context menu (cr-menu) item, leading to a false test positives. Issue 871771 fixed the ContextMenu/FilesAppBrowserTest case. An assay of the code found the remaining places that need the same fix: *) file context menu https://cs.chromium.org/search/?q=file-context-menu+file:%5Esrc/ui/file_manager/integration_tests/file_manager/+package:%5Echromium$&type=cs *) directory menu https://cs.chromium.org/chromium/src/ui/file_manager/integration_tests/file_manager/directory_tree_context_menu.js?type=cs&q=directory-tree-context-menu++file:%5Esrc/ui/file_manager/integration_tests/file_manager/+package:%5Echromium$&g=0&l=132 *) task menu lacking menu visibility check https://cs.chromium.org/search/?q=tasks-menu++file:%5Esrc/ui/file_manager/integration_tests/file_manager/+package:%5Echromium$&type=cs *) share menu needs menu selector and visibility https://cs.chromium.org/chromium/src/ui/file_manager/integration_tests/file_manager/share_and_manage_dialog.js?type=cs&q=share-menu++file:%5Esrc/ui/file_manager/integration_tests/file_manager/+package:%5Echromium$&g=0&l=27
,
Aug 8
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c60dd3dcb2ee859e555ed62793ccf4b6b9451a3e commit c60dd3dcb2ee859e555ed62793ccf4b6b9451a3e Author: Noel Gordon <noel@chromium.org> Date: Thu Aug 09 02:42:28 2018 Avoid test false positives: #directory-tree-context-menu:not([hidden]) Use the menu #directory-tree-context-menu:not([hidden]) prefix when we execute directory tree menu commands: avoids CSS matching hidden menus that have identical command |id| (e.g., cut/copy/paste). Minor: add chrome.test.assertTrues to verify remote call return values for the #directory-tree-context-menu focus/click operations. Bug: 872119 Change-Id: I6ae20e3f9ae20e9826bc16a97afbfa97a1b8b46f Reviewed-on: https://chromium-review.googlesource.com/1167345 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#581769} [modify] https://crrev.com/c60dd3dcb2ee859e555ed62793ccf4b6b9451a3e/ui/file_manager/integration_tests/file_manager/directory_tree_context_menu.js
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cc4698162c1a2a7e6253f2a5189bdb6f8c49d690 commit cc4698162c1a2a7e6253f2a5189bdb6f8c49d690 Author: Noel Gordon <noel@chromium.org> Date: Thu Aug 09 04:35:17 2018 Avoid test false positives: #tasks-menu:not([hidden]) Use the menu #tasks-menu:not([hidden]) prefix to ensure that the menu is visually displayed. Bug: 872119 Change-Id: Ia601f3e601b7a243d7613f798696da09763fc308 Reviewed-on: https://chromium-review.googlesource.com/1167346 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#581785} [modify] https://crrev.com/cc4698162c1a2a7e6253f2a5189bdb6f8c49d690/ui/file_manager/integration_tests/file_manager/tasks.js
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17a35d981aa27f3af2ae1be18cbe3a13bafd5d08 commit 17a35d981aa27f3af2ae1be18cbe3a13bafd5d08 Author: Noel Gordon <noel@chromium.org> Date: Thu Aug 09 04:47:13 2018 Avoid test false positives: #file-context-menu:not([hidden]) Use the menu #file-context-menu prefix in file menu command queries to avoids CSS matching hidden menus that have identical command |id|, for example cut/copy/paste. Minor: zip_files.js's #zip-selection menu case is specific enough that we don't need the :not([hidden]) if we add remote call result checking to the zip test code to verify #zip-selection command clicks. Remove old TODO(), reviewers advised the issue was fixed with the meta data mambo jambo cleanup/changes/fixes (see issue 867974 ). Bug: 872119 No-try: true Change-Id: I611f3a5403cbe724fa0687204c046ed2e6fffd89 Reviewed-on: https://chromium-review.googlesource.com/1167344 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#581787} [modify] https://crrev.com/17a35d981aa27f3af2ae1be18cbe3a13bafd5d08/ui/file_manager/integration_tests/file_manager/delete.js [modify] https://crrev.com/17a35d981aa27f3af2ae1be18cbe3a13bafd5d08/ui/file_manager/integration_tests/file_manager/zip_files.js
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6fdbc7b877ea46bb9a808364a4acaca6a7b6ca11 commit 6fdbc7b877ea46bb9a808364a4acaca6a7b6ca11 Author: Noel Gordon <noel@chromium.org> Date: Thu Aug 09 05:42:07 2018 Avoid test false positives: #share-menu:not([hidden]) Use the menu #share-menu:not([hidden]) prefix in the share button/menu dialog test case flow to avoid CSS matching hidden menus that have the same command |id|. None do at the moment, but the |id| "#share" is too generic, so a more precise CSS selector prevents future bugs (when we have something else to "#share" in future, which seems plausible). Minor: remove old TODO and its associated test step: the mocked shared dialog is 300 x 100px, no need to measure/worry about its size. Minor: use !!result when checking remote call return results. Bug: 872119 No-try: true Change-Id: I504421ebd4c2e9a6d0cece351b9f2bed8defde35 Reviewed-on: https://chromium-review.googlesource.com/1167924 Reviewed-by: Noel Gordon <noel@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#581799} [modify] https://crrev.com/6fdbc7b877ea46bb9a808364a4acaca6a7b6ca11/ui/file_manager/integration_tests/file_manager/share_and_manage_dialog.js
,
Aug 9
|
|||
►
Sign in to add a comment |
|||
Comment 1 by noel@chromium.org
, Aug 8Labels: OS-Chrome
Owner: noel@chromium.org
Status: Started (was: Untriaged)