The test MaterialHistoryBrowserTest.HistoryToolbarFocusTest should be an interactive_ui_test |
||||
Issue description
The test is fundamentally flawed, since it expects a particular widget to be focused. Chrome focus semantics are based on the Windows platform, where a widget cannot be focused without window activation. browser_tests can be sharded, so there is no way to enforce that a given window is activated. Focus tests should be interactive_ui_tests, and they should explicitly activate the window.
Note that on macOS, windows from applications without a bundle cannot be activated by default. The following lines are necessary.
"""
98 [NSApp setActivationPolicy:NSApplicationActivationPolicyRegular];
99 [NSApp activateIgnoringOtherApps:YES];
...
browser()->window()->Show();
"""
Alternatively, the class ScopedFakeNSWindowFocus can be used to help browser_tests deal with focus issues.
,
Oct 5 2016
,
Jan 6 2017
,
Jan 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd856e843b236c65c916b86be2a32288cc0bebd5 commit bd856e843b236c65c916b86be2a32288cc0bebd5 Author: tsergeant <tsergeant@chromium.org> Date: Thu Jan 12 02:36:47 2017 MD History: Move flaky focus tests into interactive UI tests MD History has several tests which test how focus reacts to keyboard shortcuts. These have been flaky in browser tests, failing most of the time when run in parallel with other browser tests. This CL attempts to restore these tests by moving them to interactive UI tests, where they should always be run one at a time. These are the first interactive UI tests which run tests in a WebUI page, so several test files have been moved in the build to accommodate this. BUG= 642467 Review-Url: https://codereview.chromium.org/2613503004 Cr-Commit-Position: refs/heads/master@{#443122} [modify] https://crrev.com/bd856e843b236c65c916b86be2a32288cc0bebd5/chrome/browser/BUILD.gn [modify] https://crrev.com/bd856e843b236c65c916b86be2a32288cc0bebd5/chrome/test/BUILD.gn [modify] https://crrev.com/bd856e843b236c65c916b86be2a32288cc0bebd5/chrome/test/data/webui/md_history/history_list_test.js [modify] https://crrev.com/bd856e843b236c65c916b86be2a32288cc0bebd5/chrome/test/data/webui/md_history/history_synced_tabs_test.js [modify] https://crrev.com/bd856e843b236c65c916b86be2a32288cc0bebd5/chrome/test/data/webui/md_history/history_toolbar_test.js [modify] https://crrev.com/bd856e843b236c65c916b86be2a32288cc0bebd5/chrome/test/data/webui/md_history/md_history_browsertest.js [add] https://crrev.com/bd856e843b236c65c916b86be2a32288cc0bebd5/chrome/test/data/webui/md_history/md_history_focus_test.js
,
Jan 17 2017
The CL above was sufficient to re-enable the several MD History focus tests (including the toolbar focus test) as interactive ui tests. However, there's still a workaround in web_ui_browser_test.cc to make these tests work at all. Pending CL to remove it is here: https://codereview.chromium.org/2638843002/ It involves: 1. Moving other focus-dependent WebUI tests into interactive UI tests 2. Explicitly focusing the test before it runs, as in the description. I successfully got this working on Mac with the full window activation code from the description -- however, I also found that just focusing the WebContents was enough to make the test pass. I haven't had any problems with this simpler approach either on a local mac machine or on the trybots. I guess the fact that we're only testing things inside the WebContents means we don't need the full repertoire of window activation code?
,
Jan 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb7b4ef61424259ad589b83efa6dd41e682299cf commit fb7b4ef61424259ad589b83efa6dd41e682299cf Author: tsergeant <tsergeant@chromium.org> Date: Wed Jan 18 23:12:09 2017 MD WebUI: Remove WebUI test focus hack by moving to interactive ui tests WebUIBrowserTest currently has a workaround to focus the test window when starting a test. However, tests which rely on focus will still often fail when run in parallel with other tests. This CL removes this workaround and moves the affected tests to interactive ui tests, which run serially and are able to reliably focus their WebContents before running. BUG= 642467 Review-Url: https://codereview.chromium.org/2638843002 Cr-Commit-Position: refs/heads/master@{#444536} [modify] https://crrev.com/fb7b4ef61424259ad589b83efa6dd41e682299cf/chrome/test/BUILD.gn [modify] https://crrev.com/fb7b4ef61424259ad589b83efa6dd41e682299cf/chrome/test/base/web_ui_browser_test.cc [modify] https://crrev.com/fb7b4ef61424259ad589b83efa6dd41e682299cf/chrome/test/data/webui/cr_elements/cr_action_menu_test.js [modify] https://crrev.com/fb7b4ef61424259ad589b83efa6dd41e682299cf/chrome/test/data/webui/cr_elements/cr_elements_browsertest.js [add] https://crrev.com/fb7b4ef61424259ad589b83efa6dd41e682299cf/chrome/test/data/webui/cr_elements/cr_elements_focus_test.js [modify] https://crrev.com/fb7b4ef61424259ad589b83efa6dd41e682299cf/chrome/test/data/webui/cr_elements/cr_profile_avatar_selector_tests.js [modify] https://crrev.com/fb7b4ef61424259ad589b83efa6dd41e682299cf/chrome/test/data/webui/md_history/md_history_focus_test.js [add] https://crrev.com/fb7b4ef61424259ad589b83efa6dd41e682299cf/chrome/test/data/webui/polymer_interactive_ui_test.js
,
Jan 19 2017
This should now be fixed. I'll keep an eye on the flakiness dashboard over the next week to make sure everything is working as expected. |
||||
►
Sign in to add a comment |
||||
Comment 1 by dbeam@chromium.org
, Oct 5 2016