Skip to content

Commit

Permalink
Remove deprecated ThreadTest::GetThreadTaskRunner and use the newer C…
Browse files Browse the repository at this point in the history 
…reateNewThread API. (flutter#11395)

We will end up creating fewer threads in tests.
  • Loading branch information
@chinmaygarde
chinmaygarde authored Aug 23, 2019
1 parent 72283b6 commit b7d0313
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 74 deletions.
74 changes: 38 additions & 36 deletions lib/ui/painting/image_decoder_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,12 @@ static sk_sp<SkData> OpenFixtureAsSkData(const char* name) {

TEST_F(ImageDecoderFixtureTest, CanCreateImageDecoder) {
auto loop = fml::ConcurrentMessageLoop::Create();
TaskRunners runners(GetCurrentTestName(), // label
GetThreadTaskRunner(), // platform
GetThreadTaskRunner(), // gpu
GetThreadTaskRunner(), // ui
GetThreadTaskRunner() // io
auto thread_task_runner = CreateNewThread();
TaskRunners runners(GetCurrentTestName(), // label
thread_task_runner, // platform
thread_task_runner, // gpu
thread_task_runner, // ui
thread_task_runner // io

);

Expand All @@ -125,15 +126,16 @@ TEST_F(ImageDecoderFixtureTest, CanCreateImageDecoder) {

TEST_F(ImageDecoderFixtureTest, InvalidImageResultsError) {
auto loop = fml::ConcurrentMessageLoop::Create();
TaskRunners runners(GetCurrentTestName(), // label
GetThreadTaskRunner(), // platform
GetThreadTaskRunner(), // gpu
GetThreadTaskRunner(), // ui
GetThreadTaskRunner() // io
auto thread_task_runner = CreateNewThread();
TaskRunners runners(GetCurrentTestName(), // label
thread_task_runner, // platform
thread_task_runner, // gpu
thread_task_runner, // ui
thread_task_runner // io
);

fml::AutoResetWaitableEvent latch;
GetThreadTaskRunner()->PostTask([&]() {
thread_task_runner->PostTask([&]() {
TestIOManager manager(runners.GetIOTaskRunner());
ImageDecoder decoder(runners, loop->GetTaskRunner(),
manager.GetWeakIOManager());
Expand All @@ -155,11 +157,11 @@ TEST_F(ImageDecoderFixtureTest, InvalidImageResultsError) {

TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) {
auto loop = fml::ConcurrentMessageLoop::Create();
TaskRunners runners(GetCurrentTestName(), // label
GetThreadTaskRunner(), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
TaskRunners runners(GetCurrentTestName(), // label
CreateNewThread("platform"), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
);

fml::AutoResetWaitableEvent latch;
Expand Down Expand Up @@ -197,11 +199,11 @@ TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) {

TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) {
auto loop = fml::ConcurrentMessageLoop::Create();
TaskRunners runners(GetCurrentTestName(), // label
GetThreadTaskRunner(), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
TaskRunners runners(GetCurrentTestName(), // label
CreateNewThread("platform"), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
);

fml::AutoResetWaitableEvent latch;
Expand Down Expand Up @@ -244,11 +246,11 @@ TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) {

TEST_F(ImageDecoderFixtureTest, CanDecodeWithoutAGPUContext) {
auto loop = fml::ConcurrentMessageLoop::Create();
TaskRunners runners(GetCurrentTestName(), // label
GetThreadTaskRunner(), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
TaskRunners runners(GetCurrentTestName(), // label
CreateNewThread("platform"), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
);

fml::AutoResetWaitableEvent latch;
Expand Down Expand Up @@ -295,11 +297,11 @@ TEST_F(ImageDecoderFixtureTest, CanDecodeWithResizes) {
ASSERT_NE(image_dimensions.width(), image_dimensions.height());

auto loop = fml::ConcurrentMessageLoop::Create();
TaskRunners runners(GetCurrentTestName(), // label
GetThreadTaskRunner(), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
TaskRunners runners(GetCurrentTestName(), // label
CreateNewThread("platform"), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
);

fml::AutoResetWaitableEvent latch;
Expand Down Expand Up @@ -380,11 +382,11 @@ TEST_F(ImageDecoderFixtureTest, CanResizeWithoutDecode) {
ASSERT_NE(image_dimensions.width(), image_dimensions.height());

auto loop = fml::ConcurrentMessageLoop::Create();
TaskRunners runners(GetCurrentTestName(), // label
GetThreadTaskRunner(), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
TaskRunners runners(GetCurrentTestName(), // label
CreateNewThread("platform"), // platform
CreateNewThread("gpu"), // gpu
CreateNewThread("ui"), // ui
CreateNewThread("io") // io
);

fml::AutoResetWaitableEvent latch;
Expand Down
8 changes: 4 additions & 4 deletions runtime/dart_isolate_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ TEST_F(DartIsolateTest, CanRegisterNativeCallback) {
})));
const auto settings = CreateSettingsForFixture();
auto vm_ref = DartVMRef::Create(settings);
auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(),
auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(),
"canRegisterNativeCallback", {});
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
Expand All @@ -350,7 +350,7 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) {

const auto settings = CreateSettingsForFixture();
auto vm_ref = DartVMRef::Create(settings);
auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(),
auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(),
"testCanSaveCompilationTrace", {});
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
Expand All @@ -373,7 +373,7 @@ TEST_F(DartIsolateTest, CanLaunchSecondaryIsolates) {
})));
const auto settings = CreateSettingsForFixture();
auto vm_ref = DartVMRef::Create(settings);
auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(),
auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(),
"testCanLaunchSecondaryIsolate", {});
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
Expand All @@ -392,7 +392,7 @@ TEST_F(DartIsolateTest, CanRecieveArguments) {

const auto settings = CreateSettingsForFixture();
auto vm_ref = DartVMRef::Create(settings);
auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(),
auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(),
"testCanRecieveArguments", {"arg1"});
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
Expand Down
3 changes: 2 additions & 1 deletion runtime/dart_lifecycle_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ TEST_F(DartLifecycleTest, DISABLED_ShuttingDownTheVMShutsDownAllIsolates) {
// Make sure the service protocol launches
settings.enable_observatory = true;

auto thread_task_runner = CreateNewThread();

for (size_t i = 0; i < 3; i++) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());

Expand All @@ -127,7 +129,6 @@ TEST_F(DartLifecycleTest, DISABLED_ShuttingDownTheVMShutsDownAllIsolates) {

fml::CountDownLatch latch(isolate_count);
auto vm_data = vm_ref.GetVMData();
auto thread_task_runner = GetThreadTaskRunner();
for (size_t i = 0; i < isolate_count; ++i) {
thread_task_runner->PostTask(
[vm_data, &settings, &latch, thread_task_runner]() {
Expand Down
10 changes: 5 additions & 5 deletions shell/common/shell_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ TEST_F(ShellTest, WaitForFirstFrameMultiple) {
/// single-thread setup.
TEST_F(ShellTest, WaitForFirstFrameInlined) {
Settings settings = CreateSettingsForFixture();
auto task_runner = GetThreadTaskRunner();
auto task_runner = CreateNewThread();
TaskRunners task_runners("test", task_runner, task_runner, task_runner,
task_runner);
std::unique_ptr<Shell> shell =
Expand Down Expand Up @@ -617,7 +617,7 @@ static size_t GetRasterizerResourceCacheBytesSync(Shell& shell) {

TEST_F(ShellTest, SetResourceCacheSize) {
Settings settings = CreateSettingsForFixture();
auto task_runner = GetThreadTaskRunner();
auto task_runner = CreateNewThread();
TaskRunners task_runners("test", task_runner, task_runner, task_runner,
task_runner);
std::unique_ptr<Shell> shell =
Expand Down Expand Up @@ -667,7 +667,7 @@ TEST_F(ShellTest, SetResourceCacheSize) {

TEST_F(ShellTest, SetResourceCacheSizeEarly) {
Settings settings = CreateSettingsForFixture();
auto task_runner = GetThreadTaskRunner();
auto task_runner = CreateNewThread();
TaskRunners task_runners("test", task_runner, task_runner, task_runner,
task_runner);
std::unique_ptr<Shell> shell =
Expand Down Expand Up @@ -695,7 +695,7 @@ TEST_F(ShellTest, SetResourceCacheSizeEarly) {

TEST_F(ShellTest, SetResourceCacheSizeNotifiesDart) {
Settings settings = CreateSettingsForFixture();
auto task_runner = GetThreadTaskRunner();
auto task_runner = CreateNewThread();
TaskRunners task_runners("test", task_runner, task_runner, task_runner,
task_runner);
std::unique_ptr<Shell> shell =
Expand Down Expand Up @@ -733,7 +733,7 @@ TEST_F(ShellTest, SetResourceCacheSizeNotifiesDart) {

TEST_F(ShellTest, CanCreateImagefromDecompressedBytes) {
Settings settings = CreateSettingsForFixture();
auto task_runner = GetThreadTaskRunner();
auto task_runner = CreateNewThread();

TaskRunners task_runners("test", task_runner, task_runner, task_runner,
task_runner);
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/embedder/tests/embedder_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ TEST_F(EmbedderTest, PlatformMessagesCanReceiveResponse) {
};
Captures captures;

GetThreadTaskRunner()->PostTask([&]() {
CreateNewThread()->PostTask([&]() {
captures.thread_id = std::this_thread::get_id();
auto& context = GetEmbedderContext();
EmbedderConfigBuilder builder(context);
Expand Down
9 changes: 0 additions & 9 deletions testing/thread_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,12 @@ namespace testing {

// |testing::Test|
void ThreadTest::SetUp() {
thread_ = std::make_unique<fml::Thread>();
thread_task_runner_ = thread_->GetTaskRunner();

fml::MessageLoop::EnsureInitializedForCurrentThread();
current_task_runner_ = fml::MessageLoop::GetCurrent().GetTaskRunner();
}

// |testing::Test|
void ThreadTest::TearDown() {
thread_task_runner_ = nullptr;
thread_ = nullptr;
current_task_runner_ = nullptr;
extra_threads_.clear();
}
Expand All @@ -30,10 +25,6 @@ fml::RefPtr<fml::TaskRunner> ThreadTest::GetCurrentTaskRunner() {
return current_task_runner_;
}

fml::RefPtr<fml::TaskRunner> ThreadTest::GetThreadTaskRunner() {
return thread_task_runner_;
}

fml::RefPtr<fml::TaskRunner> ThreadTest::CreateNewThread(std::string name) {
auto thread = std::make_unique<fml::Thread>(name);
auto runner = thread->GetTaskRunner();
Expand Down
18 changes: 0 additions & 18 deletions testing/thread_test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,6 @@ class ThreadTest : public ::testing::Test {
///
fml::RefPtr<fml::TaskRunner> GetCurrentTaskRunner();

//----------------------------------------------------------------------------
/// @brief Get the task runner for a dedicated thread created for this
/// test instance by this fixture. This threads message loop will
/// be terminated and the thread joined when the test is done.
///
/// @attention Prefer using the `CreateNewThread` call.
///
/// @bug This is an older call that should probably be deprecated and
/// eventually removed from use. It is redundant now that the
/// `CreateNewThread` call allows tests to create new threads (and
/// name them to boot) as necessary.
///
/// @return The task runner for a dedicated thread created for the test.
///
fml::RefPtr<fml::TaskRunner> GetThreadTaskRunner();

//----------------------------------------------------------------------------
/// @brief Creates a new thread, initializes a message loop on it, and,
/// returns its task runner to the unit-test. The message loop is
Expand All @@ -80,8 +64,6 @@ class ThreadTest : public ::testing::Test {
void TearDown() override;

private:
std::unique_ptr<fml::Thread> thread_;
fml::RefPtr<fml::TaskRunner> thread_task_runner_;
fml::RefPtr<fml::TaskRunner> current_task_runner_;
std::vector<std::unique_ptr<fml::Thread>> extra_threads_;
};
Expand Down

0 comments on commit b7d0313

Please sign in to comment.