The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/7ab03b1282bf1526e3b3619253d202d9db0fd8c3
commit 7ab03b1282bf1526e3b3619253d202d9db0fd8c3
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Wed Aug 09 18:32:37 2017
Update ScopedTaskEnvironment to be resilient to tests quitting the RunLoop
Various tests (such as those using PipelineIntegrationTestBase::Seek()
and similar) use the (deprecated) MessageLoop::QuitWhenIdleClosure() or
RunLoop::QuitCurrentWhenIdleDeprecated().
That usage was previously incompatible with
ScopedTaskEnvironment::RunUntilIdle(), which expects its run_loop to
execute the OnQueueEmptyClosure (which is a OnceClosure). This was
causing things like media_pipeline_integration_fuzzer's ::Seek() to
crash due to the Seek completion having quit the run_loop without
running the ScopedTaskEnvironment's OnQueueEmptyClosure.
This change implements fdoray@'s suggestion (in lieu of fixing all
the related deprecated test usages for now) to just restart
ScopedTaskEnvironment::RunUntilIdle() in such cases.
BUG= 751011 ,708584
R=fdoray@chromium.org
TEST=Locally on linux, crashing fuzzer case runs to completion, and no media_unittests regression
Change-Id: Id99fe93e1eddfaa83bc90fa1ac61ce96c5ac6999
Reviewed-on: https://chromium-review.googlesource.com/604504
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493066}
[modify] https://crrev.com/7ab03b1282bf1526e3b3619253d202d9db0fd8c3/base/test/scoped_task_environment.cc
Hi!
In CL:
commit 86573e43dc92125878dcafc820c5dce12876317e
Review-Url: https://codereview.chromium.org/2800893002
MessageLoop was replaced by ScopedTaskEnvironment in many classes from "/content/renderer/media_recorder/" but not in "content/renderer/media_recorder/video_track_recorder_unittest.cc".
I found that in video_track_recorder_unittest.cc the testing code checks "message_loop_.IsCurrent()" but ScopedTaskEnvironment has no IsCurrent() method. What are your plan? Are you going to add it or replace IsCurrent() in tests?
I urge MessageLoop to be removed in video_track_recorder_unittest.cc because we are getting "[FATAL:scoped_task_environment.cc(95)] Check failed: !TaskScheduler::GetInstance(). " when we run MessageLooped test and ScopedTaskEnvironmented test one by one.
For example, run: out/Debug/bin/run_content_unittests gtest -v --output-directory src/out/Debug --num-retries=0 --gtest-filter=VideoTrackRecorderTest.ConstructAndDestruct/5:WebContentsAudioInputStreamTest.MirroringNothing/1 and you will see.
Hi! Just want to add a strong +1 for adding mock time to the ScopedTaskEnvironment. I have a bunch of tests that would love to combine nested RunLoop with mock time and I think that this is the way forward for a solution to that?
To make matters worse, I have seen tests which explicitly wait on a RunLoop without any time forwarding, so the test just sits until the {delayed task, clock, etc} fires! These tests could be trivially fixed with a ScopedTaskEnvironment that supports mock time (I think).
Glad to hear the enthusiasm around ScopedTaskEnvironment's MOCK_TIME mode :).
Indeed the goal is to make it trivial to have time fast-forward when there's only delayed work remaining during RunLoop::Run().
The MOCK_TIME mode just landed and supports that on the main thread of ScopedTaskEnvironment.
I'm now working on two improvements:
1) Allow MOCK_TIME mode for MainThreadType::UI/IO as well.
2) Let MOCK_TIME mode take effect across all threads (i.e. TaskScheduler threads and main thread).
Eventually I'd like to make MOCK_TIME the default mode in unit tests!
PS: Apologies for the slow pace, I'm technically on leave and only working on this in my spare time.
