Skip to content

Commit

Permalink
Revert "[base] Ensure that tests don't change the thread priority."
Browse files Browse the repository at this point in the history
This reverts commit a4dd23f.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=1004879

Original change's description:
> [base] Ensure that tests don't change the thread priority.
> 
> 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).
> 
> Bug: 931706
> Change-Id: I5ca18b87720b5305285be481f7b60ff5c941a324
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1804786
> Commit-Queue: Scott Violet <sky@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Auto-Submit: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#697261}

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

Change-Id: I900562585a34cd92a4f49a215d7c323706ce468d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 931706,1004879
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1808814
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697316}
  • Loading branch information
zhenyao authored and Commit Bot committed Sep 17, 2019
1 parent df85eae commit 523861d
Show file tree
Hide file tree
Showing 11 changed files with 11 additions and 65 deletions.
5 changes: 2 additions & 3 deletions ash/shell/content/test/ash_content_perf_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ class AshContentTestSuite : public content::ContentTestSuiteBase {
protected:
// content::ContentTestSuiteBase:
void Initialize() override {
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals. (Must run
// before the base class is initialized.)
base::TestSuite::DisableCheckForLeakedGlobals();
base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();
ContentTestSuiteBase::Initialize();
ui_controls::InstallUIControlsAura(ash::test::CreateAshUIControls());
}
Expand Down
33 changes: 0 additions & 33 deletions base/test/test_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#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"
Expand Down Expand Up @@ -189,31 +188,6 @@ 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();
Expand Down Expand Up @@ -407,11 +381,6 @@ void TestSuite::DisableCheckForProcessPriority() {
check_for_process_priority_ = false;
}

void TestSuite::DisableCheckForThreadPriorityAtTestEnd() {
DCHECK(!is_initialized_);
check_for_thread_priority_at_test_end_ = false;
}

void TestSuite::UnitTestAssertHandler(const char* file,
int line,
const StringPiece summary,
Expand Down Expand Up @@ -602,8 +571,6 @@ void TestSuite::Initialize() {
if (check_for_process_priority_)
listeners.Append(new CheckProcessPriority);
#endif
listeners.Append(
new CheckThreadPriority(check_for_thread_priority_at_test_end_));

AddTestLauncherResultPrinter();

Expand Down
5 changes: 0 additions & 5 deletions base/test/test_suite.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ class TestSuite {
// Disables checks for process priority. Most tests should not use this.
void DisableCheckForProcessPriority();

// Disables checks for thread priority at test end. This may be used for tests
// that each run in their own process.
void DisableCheckForThreadPriorityAtTestEnd();

// Disables checks for certain global objects being leaked across tests.
void DisableCheckForLeakedGlobals();

Expand Down Expand Up @@ -98,7 +94,6 @@ class TestSuite {

bool check_for_leaked_globals_ = true;
bool check_for_process_priority_ = true;
bool check_for_thread_priority_at_test_end_ = true;

bool is_initialized_ = false;

Expand Down
4 changes: 1 addition & 3 deletions chrome/test/base/browser_tests_main_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ 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 and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}
};
Expand Down
4 changes: 1 addition & 3 deletions chrome/test/base/chrome_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ ChromeTestSuiteRunner::~ChromeTestSuiteRunner() {}

int ChromeTestSuiteRunner::RunTestSuite(int argc, char** argv) {
ChromeTestSuite test_suite(argc, argv);
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
#if defined(OS_ANDROID)
// Android browser tests run child processes as threads instead.
content::ContentTestSuiteBase::RegisterInProcessThreads();
Expand Down
4 changes: 1 addition & 3 deletions chrome/test/base/interactive_ui_tests_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ class InteractiveUITestSuite : public ChromeTestSuite {
protected:
// ChromeTestSuite overrides:
void Initialize() override {
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals.
base::TestSuite::DisableCheckForLeakedGlobals();
base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();

ChromeTestSuite::Initialize();

Expand Down
4 changes: 1 addition & 3 deletions chromecast/app/cast_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ 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 and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}

Expand Down
5 changes: 2 additions & 3 deletions content/test/content_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@ class ContentBrowserTestSuite : public ContentTestSuiteBase {

protected:
void Initialize() override {
// Browser tests are expected not to tear-down various globals and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals. (Must run
// before the base class is initialized.)
base::TestSuite::DisableCheckForLeakedGlobals();
base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();

ContentTestSuiteBase::Initialize();

Expand Down
4 changes: 1 addition & 3 deletions extensions/shell/test/shell_test_launcher_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ 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 and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}

Expand Down
4 changes: 1 addition & 3 deletions fuchsia/engine/test/web_engine_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ 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 and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}

Expand Down
4 changes: 1 addition & 3 deletions headless/test/headless_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ 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 and may
// complete with the thread priority being above NORMAL.
// Browser tests are expected not to tear-down various globals.
test_suite.DisableCheckForLeakedGlobals();
test_suite.DisableCheckForThreadPriorityAtTestEnd();
return test_suite.Run();
}

Expand Down

0 comments on commit 523861d

Please sign in to comment.