Various events/mouse- layout tests are flaky when run in parallel |
|||||||
Issue descriptionFrom what I can tell, a bunch of fast/events/mouse-* tests and the same tests in two virtual test suites *always* fail when they are run in parallel in the first main pass of the layout tests, but they tend to succeed when they are retried. I'm not sure if they're being run concurrently and that's the trouble, or if they're interacting with other tests, or what; this needs further investigation. However, I'm marking them as flaky for now to clean up the output from run-webkit-tests. Rick, can you point this bug at the right people to look at it?
,
Mar 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/caa69c87a3f23589af4534daf985d838a0a59de7 commit caa69c87a3f23589af4534daf985d838a0a59de7 Author: dpranke <dpranke@chromium.org> Date: Thu Mar 03 23:41:26 2016 Mark some mouse event layout tests as flaky. TBR=rbyers@chromium.org BUG= 591821 Review URL: https://codereview.chromium.org/1761093002 Cr-Commit-Position: refs/heads/master@{#379138} [modify] https://crrev.com/caa69c87a3f23589af4534daf985d838a0a59de7/third_party/WebKit/LayoutTests/TestExpectations
,
Mar 4 2016
Thanks Dirk. Navid, can look into this at some point? One similar issue I've seen in the past was due to EventHandler::clear not correctly resetting some state and so state setup by a previous test would impact another test.
,
Mar 4 2016
Sure. I'll take a look.
,
Mar 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c975dce2934e94db8aef1dfa46a6598d6d20f769 commit c975dce2934e94db8aef1dfa46a6598d6d20f769 Author: dpranke <dpranke@chromium.org> Date: Sun Mar 06 02:33:07 2016 Update TestExpectations, SlowTests for various passes and failures. Unreviewed, expectations change. TBR=qyearsley@chromium.org BUG= 243871 , 310679 , 321237 , 331582 , 404597 ,412381, 441840 , 463798 ,490511, 491764 , 501229 , 501659 , 510337 , 542660 , 570702 , 582836 , 583675 , 584807 , 587950 , 591821 ,592183, 592185 Review URL: https://codereview.chromium.org/1763323002 Cr-Commit-Position: refs/heads/master@{#379482} [modify] https://crrev.com/c975dce2934e94db8aef1dfa46a6598d6d20f769/third_party/WebKit/LayoutTests/SlowTests [modify] https://crrev.com/c975dce2934e94db8aef1dfa46a6598d6d20f769/third_party/WebKit/LayoutTests/TestExpectations
,
Mar 9 2016
dpranke@ were you able to make them fail locally or was it only on the bots? Were you running on the linux and what was the command that you were running and seeing this failure? I tried running them locally on my Linux with -f option for parallel but they seem to always pass.
,
Mar 9 2016
Yes, they fail initially for me locally, but pass on the retry. You can see the flakiness on the bots, e.g.: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast/events/mouse-click-events.html https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/61776/steps/webkit_tests/logs/stdio (in the latter log, search for "retrying 50 unexpected failures" I was not running with -f, however, and haven't tried that.
,
Mar 9 2016
I still couldn't reproduce the problem. The only one that was reproducible was fast/events/mouse-event-buttons-attribute.html and I have the fix up for that one. https://codereview.chromium.org/1775613004/ Can you help me repoduce the problem locally for the rest of them? When I run the mouse tests they all pass in my local machine. Here is what I did. I removed those lines from TestExpectation file and ran this command: ./third_party/WebKit/Tools/Scripts/run-webkit-tests fast/events/mouse* -f --repeat-each=1000
,
Mar 9 2016
Don't run fast/events/mouse by itself, that probably eliminates the concurrency and makes the problem go away. Run the entire test suite (i.e., run run-webkit-tests with no arguments at all).
,
Mar 10 2016
Thanks. I was able to reproduce the problem. But the cause of the problem I'm seeing is a bit weird. Take fast/events/mouse-down-on-pseudo-element-remove-crash.html for example. It is a very simple test that only sends a few events and wrote something in the document body. When I run it, it fails on the line that writes something in the body: document.body.textContent = 'PASS; NOT CRASHED'; and it says Cannot set property 'textContent' of null. It is as if the document body is not fully created or something. I also put the whole process in the onload function of the window but still the same result. I can see the same behavior in other tests too that as if there are no elements to get the events or they get these null errors. I was wondering if you have seen something like this before or if you have any idea about possible cause of that.
,
Mar 10 2016
I have certainly seen inexplicable flakiness before that suggests that something has gone horribly wrong. I do not have any immediate theories as to the possible cause in this case.
,
Mar 11 2016
dpranke@ here is my new findings after a few days of debugging :) It seems that there is a state in the content shell that leaks to the other tests. Basically there is this test: fast/events/hit-test-counts that calls window.internals.settings.setViewportEnabled. Then the content_shell which has run this test will keep this state for the rest of the tests that will be run on it and it causes them to fail. Basically what happens is that the mouse event positions will be calculated incorrectly as the result of this state. I also found Issue 571233 which is related this this API. Do you know if that issue is being investigated or what?
,
Mar 11 2016
That sounds like exactly the sort of bug I'd expect to cause these sorts of failures. I know nothing about the state of that bug, but hopefully skobes@ can chime in :).
,
Mar 11 2016
This is clearly a general problem with WebSettings, not just setViewportEnabled: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/public/web/WebSettings.h&q=setViewportEnabled&sq=package:chromium&type=cs&l=246 Issue 537764 seems to be addressing the general problem. wkorman@, did I get it right?
,
Mar 11 2016
Yes, that bug will provide a nice way to solve it generally. However in the meantime if there is state that's not cleaned up between tests it is possible to add it to the right place in the layout test runner reset/cleanup code. See for example: https://code.google.com/p/chromium/codesearch#chromium/src/content/shell/renderer/layout_test/blink_test_runner.cc&sq=package:chromium&type=cs&l=850&rcl=1457713833
,
Mar 11 2016
I haven't looked into issue 571233 yet. I didn't realize it affected other tests besides background-color-outside-document.html.
,
Mar 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c2c57c405548a45113c00b198afa74dcacc982a2 commit c2c57c405548a45113c00b198afa74dcacc982a2 Author: skobes <skobes@chromium.org> Date: Sun Mar 13 02:09:42 2016 Clear page-defined constraints when viewportEnabled setting becomes false. Without this, the values in PageScaleConstraintsSet (and things derived from it such as the main frame layout size) remain stale after the setting flip. Dev tools emulation avoided this issue by calling WebViewImpl::disableViewport, but the layout test runner uses auto-generated code to clear the viewportEnabled setting (InternalSettingsGenerated::resetToConsistentState). With this change, DevToolsEmulator can also tweak the setting directly. BUG= 571233 , 591821 Review URL: https://codereview.chromium.org/1787913002 Cr-Commit-Position: refs/heads/master@{#380888} [modify] https://crrev.com/c2c57c405548a45113c00b198afa74dcacc982a2/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/c2c57c405548a45113c00b198afa74dcacc982a2/third_party/WebKit/Source/web/DevToolsEmulator.cpp [modify] https://crrev.com/c2c57c405548a45113c00b198afa74dcacc982a2/third_party/WebKit/Source/web/WebViewImpl.cpp [modify] https://crrev.com/c2c57c405548a45113c00b198afa74dcacc982a2/third_party/WebKit/Source/web/WebViewImpl.h
,
Mar 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/228ea6faf71a8e33938caf6d2c04f80ae786dbf2 commit 228ea6faf71a8e33938caf6d2c04f80ae786dbf2 Author: nzolghadr <nzolghadr@chromium.org> Date: Tue Mar 15 12:14:56 2016 Remove fixed tests from TestExpectation file These tests were failing because of a state that was not reset in the content_shell and it was being carried over from other tests and resulting these test to fail. The blocking bug is now fixed and that state is being reset so these tests are always passing now. Marking the rest of the tests for another bug as they seem to have another reason for failing. BUG= 591821 , 594672 Review URL: https://codereview.chromium.org/1798963003 Cr-Commit-Position: refs/heads/master@{#381211} [modify] https://crrev.com/228ea6faf71a8e33938caf6d2c04f80ae786dbf2/third_party/WebKit/LayoutTests/TestExpectations
,
Mar 15 2016
,
May 18 2016
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by dpranke@chromium.org
, Mar 3 2016