Issue metadata
Sign in to add a comment
|
KeyboardControllerTest.FloatingKeyboardShowOnFirstTap failure on Win x64 slipped past CQ |
||||||||||||||||||||||
Issue descriptionhttps://codereview.chromium.org/2620273002/ had a failure of KeyboardControllerTest.FloatingKeyboardShowOnFirstTap on the win_chromium_x64_rel_ng tryserver. For some reason, the test also failed without the patch: https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/347001 and this caused the CQ to commit the patch when it shouldn't have. At that point, all subsequent builds on that tryserver started failing that test, and it started failing on the waterfall as well: https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64/builds/7528 Before re-landing that patch it must be understood how that patch slipped past the CQ.
,
Jan 13 2017
,
Jan 16 2017
,
Jan 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/485cef68ecb7672a26df161f224d1f5e4a72dfaa commit 485cef68ecb7672a26df161f224d1f5e4a72dfaa Author: yhanada <yhanada@chromium.org> Date: Mon Jan 16 16:23:27 2017 Remove implicit dependency between test cases. Accessibility keyboard enabled flags is not reset in |CloseKeyboard| test case accidentally and |FloatingKeyboardShowOnFirstTap| test depends on this behavior. This causes test failures when test execution order changes by adding test cases. TEST=n/a; no behavior change. BUG= 681007 Review-Url: https://codereview.chromium.org/2634933002 Cr-Commit-Position: refs/heads/master@{#443905} [modify] https://crrev.com/485cef68ecb7672a26df161f224d1f5e4a72dfaa/ui/keyboard/keyboard_controller_unittest.cc
,
Jan 16 2017
The test failed without the patch because it depends on a flag that is set by another test and only the test was run when retrying without patch.
,
Jan 17 2017
Thanks for fixing this. You might want to consider using RAII to enable and disable these flags, and stack allocate an object for turning them on and off, so that structurally it's impossible to write an incorrect test.
,
Jan 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3ad6fbee118e858145e28d4f5720f695069c956b commit 3ad6fbee118e858145e28d4f5720f695069c956b Author: yhanada <yhanada@chromium.org> Date: Thu Jan 19 00:23:15 2017 Clean up keyboard_unittests. - Introduce ScopedTouchKeyboardEnabler and ScopedAccessibilityKeyboardEnabler class for managing flags by using RAII. - Add dependency to ui_test_pak in keyboard_unittests. BUG= 681007 TEST=keyboard_unittests passes. Review-Url: https://codereview.chromium.org/2634153002 Cr-Commit-Position: refs/heads/master@{#444562} [modify] https://crrev.com/3ad6fbee118e858145e28d4f5720f695069c956b/ui/keyboard/BUILD.gn [modify] https://crrev.com/3ad6fbee118e858145e28d4f5720f695069c956b/ui/keyboard/keyboard_controller_unittest.cc
,
Jan 19 2017
Ken, Thanks for pointing it out!
,
Jul 7 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by kbr@chromium.org
, Jan 13 2017