Commit 4ddc16ab authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

[base] Ensure that tests don't change the thread priority (reland #2).

This is a reland of https://crrev.com/c/chromium/src/+/1865405
The difference is that we don't check thread priority in
angle_unittest, as these tests change the thread priority on
purpose.

This CL verifies that the thread priority is normal when a test process
is launched and before/after each test. The goal is to avoid having
tests that assume they run at normal priority be disabled because of
other misbehaving tests (e.g. https://crbug.com/931706).

TBR=sky@chromium.org,michaelpg@chromium.org,thakis@chromium.org,eseckler@chromium.org

Bug: 931706
Change-Id: I8fb2230cbb9b9203884d1c440696d4ce2968dcaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881707Reviewed-by: default avatarMichael Giuffrida <michaelpg@chromium.org>
Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710326}
parent 72627f4a
......@@ -34,9 +34,10 @@ class AshContentTestSuite : public content::ContentTestSuiteBase {
protected:
// content::ContentTestSuiteBase:
void Initialize() override {
// Browser tests are expected not to tear-down various globals. (Must run
// before the base class is initialized.)
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
base::TestSuite::DisableCheckForLeakedGlobals();
base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();
ContentTestSuiteBase::Initialize();
ui_controls::InstallUIControlsAura(ash::test::CreateAshUIControls());
}
......
......@@ -37,6 +37,7 @@
#include "base/test/multiprocess_test.h"
#include "base/test/test_switches.h"
#include "base/test/test_timeouts.h"
#include "base/threading/platform_thread.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -188,6 +189,31 @@ class CheckProcessPriority : public testing::EmptyTestEventListener {
};
#endif // !defined(OS_IOS)
class CheckThreadPriority : public testing::EmptyTestEventListener {
public:
CheckThreadPriority(bool check_thread_priority_at_test_end)
: check_thread_priority_at_test_end_(check_thread_priority_at_test_end) {
CHECK_EQ(base::PlatformThread::GetCurrentThreadPriority(),
base::ThreadPriority::NORMAL);
}
void OnTestStart(const testing::TestInfo& test) override {
EXPECT_EQ(base::PlatformThread::GetCurrentThreadPriority(),
base::ThreadPriority::NORMAL);
}
void OnTestEnd(const testing::TestInfo& test) override {
if (check_thread_priority_at_test_end_) {
EXPECT_EQ(base::PlatformThread::GetCurrentThreadPriority(),
base::ThreadPriority::NORMAL);
}
}
private:
const bool check_thread_priority_at_test_end_;
DISALLOW_COPY_AND_ASSIGN(CheckThreadPriority);
};
const std::string& GetProfileName() {
static const NoDestructor<std::string> profile_name([]() {
const CommandLine& command_line = *CommandLine::ForCurrentProcess();
......@@ -376,9 +402,14 @@ void TestSuite::DisableCheckForLeakedGlobals() {
check_for_leaked_globals_ = false;
}
void TestSuite::DisableCheckForProcessPriority() {
void TestSuite::DisableCheckForThreadAndProcessPriority() {
DCHECK(!is_initialized_);
check_for_process_priority_ = false;
check_for_thread_and_process_priority_ = false;
}
void TestSuite::DisableCheckForThreadPriorityAtTestEnd() {
DCHECK(!is_initialized_);
check_for_thread_priority_at_test_end_ = false;
}
void TestSuite::UnitTestAssertHandler(const char* file,
......@@ -567,10 +598,13 @@ void TestSuite::Initialize() {
listeners.Append(new ResetCommandLineBetweenTests);
if (check_for_leaked_globals_)
listeners.Append(new CheckForLeakedGlobals);
if (check_for_thread_and_process_priority_) {
listeners.Append(
new CheckThreadPriority(check_for_thread_priority_at_test_end_));
#if !defined(OS_IOS)
if (check_for_process_priority_)
listeners.Append(new CheckProcessPriority);
#endif
}
AddTestLauncherResultPrinter();
......
......@@ -43,8 +43,15 @@ class TestSuite {
int Run();
// Disables checks for process priority. Most tests should not use this.
void DisableCheckForProcessPriority();
// Disables checks for thread and process priority at the beginning and end of
// each test. Most tests should not use this.
void DisableCheckForThreadAndProcessPriority();
// Disables checks for thread priority at the end of each test (still checks
// at the beginning of each test). This should be used for tests that run in
// their own process and should start with normal priorities but are allowed
// to end with different priorities.
void DisableCheckForThreadPriorityAtTestEnd();
// Disables checks for certain global objects being leaked across tests.
void DisableCheckForLeakedGlobals();
......@@ -93,7 +100,8 @@ class TestSuite {
std::unique_ptr<logging::ScopedLogAssertHandler> assert_handler_;
bool check_for_leaked_globals_ = true;
bool check_for_process_priority_ = true;
bool check_for_thread_and_process_priority_ = true;
bool check_for_thread_priority_at_test_end_ = true;
bool is_initialized_ = false;
......
......@@ -30,8 +30,10 @@ class BrowserTestSuiteRunnerChromeOS : public ChromeTestSuiteRunner {
public:
int RunTestSuite(int argc, char** argv) override {
BrowserTestSuiteChromeOS test_suite(argc, argv);
// Browser tests are expected not to tear-down various globals.
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}
};
......
......@@ -77,8 +77,10 @@ ChromeTestSuiteRunner::~ChromeTestSuiteRunner() {}
int ChromeTestSuiteRunner::RunTestSuite(int argc, char** argv) {
ChromeTestSuite test_suite(argc, argv);
// Browser tests are expected not to tear-down various globals.
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
#if defined(OS_ANDROID)
// Android browser tests run child processes as threads instead.
content::ContentTestSuiteBase::RegisterInProcessThreads();
......
......@@ -41,8 +41,10 @@ class InteractiveUITestSuite : public ChromeTestSuite {
protected:
// ChromeTestSuite overrides:
void Initialize() override {
// Browser tests are expected not to tear-down various globals.
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
base::TestSuite::DisableCheckForLeakedGlobals();
base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();
ChromeTestSuite::Initialize();
......
......@@ -24,8 +24,10 @@ class CastTestLauncherDelegate : public content::TestLauncherDelegate {
int RunTestSuite(int argc, char** argv) override {
base::TestSuite test_suite(argc, argv);
// Browser tests are expected not to tear-down various globals.
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}
......
......@@ -39,9 +39,10 @@ class ContentBrowserTestSuite : public ContentTestSuiteBase {
protected:
void Initialize() override {
// Browser tests are expected not to tear-down various globals. (Must run
// before the base class is initialized.)
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
base::TestSuite::DisableCheckForLeakedGlobals();
base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();
ContentTestSuiteBase::Initialize();
......
......@@ -13,8 +13,10 @@ namespace extensions {
int AppShellTestLauncherDelegate::RunTestSuite(int argc, char** argv) {
base::TestSuite test_suite(argc, argv);
// Browser tests are expected not to tear-down various globals.
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}
......
......@@ -23,8 +23,10 @@ class WebEngineTestLauncherDelegate : public content::TestLauncherDelegate {
// content::TestLauncherDelegate implementation:
int RunTestSuite(int argc, char** argv) override {
base::TestSuite test_suite(argc, argv);
// Browser tests are expected not to tear-down various globals.
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}
......
......@@ -35,8 +35,9 @@ int main(int argc, char** argv) {
angle::InitTestHarness(&argc, argv);
base::TestSuite test_suite(argc, argv);
// The process priority is lowered by the constructor of tcu::ANGLEPlatform().
test_suite.DisableCheckForProcessPriority();
// The process and thread priorities are modified by
// StabilizeCPUForBenchmarking()/SetLowPriorityProcess().
test_suite.DisableCheckForThreadAndProcessPriority();
int rt = base::LaunchUnitTestsSerially(
argc, argv, base::BindOnce(&RunHelper, base::Unretained(&test_suite)));
......
......@@ -27,6 +27,10 @@ int main(int argc, char** argv) {
ANGLEProcessPerfTestArgs(&argc, argv);
base::TestSuite test_suite(argc, argv);
// The thread priority is modified by StabilizeCPUForBenchmarking().
test_suite.DisableCheckForThreadAndProcessPriority();
int rt = base::LaunchUnitTestsSerially(
argc, argv, base::BindOnce(&RunHelper, base::Unretained(&test_suite)));
return rt;
......
......@@ -24,7 +24,10 @@ int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
testing::InitGoogleMock(&argc, argv);
sh::Initialize();
base::TestSuite test_suite(argc, argv);
test_suite.DisableCheckForThreadAndProcessPriority();
int rt = base::LaunchUnitTestsSerially(
argc, argv, base::BindOnce(&RunHelper, base::Unretained(&test_suite)));
sh::Finalize();
......
......@@ -41,8 +41,10 @@ class HeadlessTestLauncherDelegate : public content::TestLauncherDelegate {
// content::TestLauncherDelegate implementation:
int RunTestSuite(int argc, char** argv) override {
base::TestSuite test_suite(argc, argv);
// Browser tests are expected not to tear-down various globals.
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}
......
......@@ -17,6 +17,7 @@ int TestLauncherDelegateImpl::RunTestSuite(int argc, char** argv) {
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment