LockScreenAppStateWebUiLockTest failures with Mojo changes |
|||||
Issue description
The failing test is (in unit_tests):
LockScreenAppStateWebUiLockTest
.LaunchActionWhenStylusRemoved_ActionClosedBeforeAnimationDone
See blocked bug for details and motivation regarding what's changing in Mojo. Likely the failures are caused by incorrect task ordering assumptions.
,
Aug 8
+jdufault - any chance you have cycles to help look into this?
,
Aug 8
And just to clarify, these failures appear when applying this CL[1]. The CL does not break any guarantees made by Mojo bindings, but it can affect the timing of arbitrary events. The failures therefore strongly imply that incorrect ordering assumptions being made somewhere, and this has been the case for all other blocking bugs which have been fixed so far. [1] https://chromium-review.googlesource.com/c/chromium/src/+/1145692
,
Aug 8
tbarzic@ worked on the stylus-related lock screen features Webui-based lock screen hasn't been used in a few milestones and we're planning to remove it from the codebase soon (m71ish), so it may be fine to just disable this test.
,
Aug 8
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/722ad8e096ad99eb0104be5c6d5c3851e316f6e0 commit 722ad8e096ad99eb0104be5c6d5c3851e316f6e0 Author: Toni Barzic <tbarzic@google.com> Date: Thu Aug 09 22:24:59 2018 In LockScreenAppState tests flush mojo after new note requests Make sure every call to SendNewNoteRequest is followed by call to FlushTrayActionForTesting, which should flush mojo pipes. For most instances this was already the case, but it was missing in few places - these were followed up with ExpectObservedStatesMatch, which did fluch of its own (to ensure the mojo pipe is flushed in other direction), which flushed the new note request as well, or were expecting mojo call to have no effect. With CL:1145692, this does not seem to be enough anymore, as flush in ExpectObservedStatesMatch ends up handling only a single mojo message. While here, replace checks for empty observed state changed with calls to ExpectObservedStatesMatch. TEST=run tets with CL:1145692 applied BUG= 872070 Change-Id: I58447ffe8315ddcd5917f0ce901ea5b2624b4e2a Reviewed-on: https://chromium-review.googlesource.com/1170026 Commit-Queue: Toni Barzic <tbarzic@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Cr-Commit-Position: refs/heads/master@{#581917} [modify] https://crrev.com/722ad8e096ad99eb0104be5c6d5c3851e316f6e0/chrome/browser/chromeos/lock_screen_apps/state_controller_unittest.cc
,
Aug 9
Thanks! |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by roc...@chromium.org
, Aug 8