lifecycle/background-change-lifecycle-count.html is flaky on windows and linux |
|||
Issue descriptionThis test is flaky on windows and linux: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=lifecycle%2Fbackground-change-lifecycle-count.html On linux, we're getting a synthetic mouse move event that's causing an additional lifecycle update.
,
Aug 24
,
Oct 10
This just ruined a cq run for me: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/win7_chromium_rel_ng/103711 Do virtual test suites need their own flaky entry?
,
Oct 10
I thought virtual inherited the non-virtual expectations but that's not the case (I just confirmed locally)! Sorry about that; I'll suppress this failure.
,
Oct 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/12c34c9a009317509c039ad3a5ad4cb88c78bd42 commit 12c34c9a009317509c039ad3a5ad4cb88c78bd42 Author: Philip Rogers <pdr@chromium.org> Date: Wed Oct 10 14:28:17 2018 Suppress virtual/threaded/.../background-change-lifecycle-count.html virtual/threaded/lifecycle/background-change-lifecycle-count.html fails on linux and windows (see: https://crbug.com/875884) and we had a non-virtual expectation for this but were missing the virtual one. This patch updates the virtual expectation. TBR=thakis@chromium.org NOTRY=true Bug: 875884 Change-Id: I20e708e67e347aeee7603408f1d9778904d2268f Reviewed-on: https://chromium-review.googlesource.com/c/1273072 Commit-Queue: Philip Rogers <pdr@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#598312} [modify] https://crrev.com/12c34c9a009317509c039ad3a5ad4cb88c78bd42/third_party/WebKit/LayoutTests/TestExpectations
,
Oct 10
I can repro this issue on a local windows device. It doesn't matter what count I set --gtest_repeat to, at least locally. The test fails with ~50% probability on the first run, and never fails on subsequent runs. This suggests a race condition during first run setup. Looking at flakiness that happens on the CQ, we see a slightly different behavior. The test either always fails [including on repeats], or never fails [including on repeats]. This implies that there is some state that is leaking between tests.
,
Oct 10
If I modify the test runner to reset content shell between tests, then the tests will occasionally fail on repeat [using --gtest_repeat=10 or --gtest_repeat=30]. The reason I didn't land the CL to always reset content shell: https://chromium-review.googlesource.com/c/chromium/src/+/1259722 is because it roughly doubles the amount of time it takes to run the tests. However, if we're only retrying a small number of tests, then we should reset content shell for better consistency.
,
Oct 10
Erik, thats great! State leaking between tests is exactly the kind of bug this test is trying to catch. I bet we have a spurious timer or task that's active between tests. If you put a stacktrace in LocalFrameView::UpdateLifecyclePhases, it should reveal all callsites. https://bugs.chromium.org/p/chromium/issues/detail?id=877132 is fixing the only flakiness we're aware of so far (the fake mouse event hover timer).
,
Oct 11
This flakiness is also caused by the fake mouse event hover timer. This timeout of 20ms: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/input/mouse_event_manager.cc?type=cs&sq=package:chromium&g=0&l=71 causes the timer to fire at non-deterministic times during the test, causing flaky failures. I don't know why this is affected by whether we reset content shell between tests. I suspect that for a new content shell, there's a lot more tasks queued at the beginning of the test, and this causes the scheduler to have different behavior.
,
Oct 12
Thanks for taking the time to find that timer Erik. This turns out to be the same issue they are working on in https://crbug.com/877132. we're removing the timer using a feature flag so it still exists right now but should disappear soon.
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5524af6018c603cd943c8f76ade1e407a5459ad7 commit 5524af6018c603cd943c8f76ade1e407a5459ad7 Author: erikchen <erikchen@chromium.org> Date: Tue Oct 16 19:21:27 2018 Add functionality to restart content shell between test runs. Many layout tests will flakily fail when run in a clean content shell, but will deterministically fail or pass when run in a reused content shell. Unfortunately, we cannot restart content shell between all tests because that significantly increases run time. [2X on Windows, 3X on Linux, 5X on macOS]. This CL adds the functionality to restart content shell between all test runs, but then only enables it for tests with --gtest_repeat or --repeat-each with a repeat count > 1. Effects: * For a local developer, this will only have an effect when using --gtest_repeat or --repeat-each. In this case, the developer is already trying to tease out some type of non-determinism between tests, and it's important to use a clean content shell. With this CL, flaky tests can be easily reproduced with --gtest_repeat, whereas they could not before. See https://bugs.chromium.org/p/chromium/issues/detail?id=894527#c5 for examples. * On the CQ, this will only have an effect on 'retry without patch' and 'retry with patch', which retry failing tests looking for flakiness. This will help the CQ better detect tests that are flaky on ToT to reduce false rejects. Change-Id: I4c04e382e733f8e1b40f4a6dde78292a41126a1b Bug: 875884, 889036 Reviewed-on: https://chromium-review.googlesource.com/c/1273810 Reviewed-by: Aleks Totic <atotic@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Erik Chen <erikchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#600077} [modify] https://crrev.com/5524af6018c603cd943c8f76ade1e407a5459ad7/content/shell/browser/layout_test/blink_test_controller.cc [modify] https://crrev.com/5524af6018c603cd943c8f76ade1e407a5459ad7/content/shell/common/layout_test/layout_test_switches.cc [modify] https://crrev.com/5524af6018c603cd943c8f76ade1e407a5459ad7/content/shell/common/layout_test/layout_test_switches.h [modify] https://crrev.com/5524af6018c603cd943c8f76ade1e407a5459ad7/third_party/blink/tools/blinkpy/web_tests/port/base.py [modify] https://crrev.com/5524af6018c603cd943c8f76ade1e407a5459ad7/third_party/blink/tools/blinkpy/web_tests/run_webkit_tests.py |
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Aug 20