WallpaperControllerTest.MojoWallpaperObserverTest is flaky |
|||||
Issue description
Sometimes fails locally when running very large numbers of tests:
gn args:
dcheck_always_on = true
is_debug = false
is_component_build = true
target_os = "chromeos"
use_goma = true
Run:
testing/xvfb.py tools/perry.py out/Default/ash_unittests > perry.log &
tail -f perry.log
See this occasionally:
[ RUN ] WallpaperControllerTest.MojoWallpaperObserverTest
../../ash/wallpaper/wallpaper_controller_unittest.cc:590: Failure
Expected: 2
To be equal to: observer.wallpaper_colors_changed_count()
Which is: 1
The test uses RunLoop::RunUntilIdle() to wait for a mojo message -- perhaps that is the problem?
https://cs.chromium.org/chromium/src/ash/wallpaper/wallpaper_controller_unittest.cc?q=WallpaperControllerTest.MojoWallpaperObserverTest&sq=package:chromium&l=586
// Enable shelf coloring will set a customized wallpaper image and change
// session state to ACTIVE, which will trigger wallpaper colors calculation.
EnableShelfColoring();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(2, observer.wallpaper_colors_changed_count());
Maybe this just needs a FlushForTesting instead? (rockot@, we can't rely on RunUntilIdle, right?)
,
Nov 17 2017
,
Nov 20 2017
That's quite possibly the issue. It's not really a concern regarding Mojo usage specifically so much as it's just a general concern in complex runtime environments with lots of stuff going on. RunUntilIdle unblocks as soon as the calling run-loop is idle, it doesn't wait for the entire system to be idle. If any of the tested behavior relies on work being done on another thread or in another process, RunUntilIdle is an unreliable tool. As an implementation detail (true in practice, but not guaranteed by APIs), RunUntilIdle is currently sufficient even for Mojo stuff, but only if everything is bound on the same single sequence.
,
Nov 28 2017
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7419e6dfb31c3cd6ba3f8758bc45478f761b28cb commit 7419e6dfb31c3cd6ba3f8758bc45478f761b28cb Author: James Cook <jamescook@chromium.org> Date: Thu Nov 30 00:40:21 2017 chromeos: Fix flaky WallpaperControllerTest.MojoWallpaperObserver The test needs to explicitly wait for the wallpaper average color calculation to finish before making assertions about observer methods being called. Clean up RunAllTasksUntilIdle() to match the version in content. I suspect this function was copy/pasted to avoid an ash dependency on content just for test utils -- it's been there at least since 2015. Don't modify the command line to enable the shelf coloring feature because it is on by default (and has been for a long time). Bug: 786266 Test: ash_unittests --gtest_shuffle --gtest_repeat=50 Change-Id: I5b8c4e0c93de4fee0a58f672c3dd9d11a6c3f42a Reviewed-on: https://chromium-review.googlesource.com/794344 Reviewed-by: Ken Rockot <rockot@chromium.org> Commit-Queue: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/heads/master@{#520345} [modify] https://crrev.com/7419e6dfb31c3cd6ba3f8758bc45478f761b28cb/ash/wallpaper/wallpaper_controller.cc [modify] https://crrev.com/7419e6dfb31c3cd6ba3f8758bc45478f761b28cb/ash/wallpaper/wallpaper_controller_unittest.cc [modify] https://crrev.com/7419e6dfb31c3cd6ba3f8758bc45478f761b28cb/mojo/public/cpp/bindings/interface_ptr_set.h
,
Nov 30 2017
,
Jul 30
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 Deleted