Skip to content

Commit

Permalink
Replaced --single_process with --single-process-tests
Browse files Browse the repository at this point in the history
Ensured that functionality that runs tests + launcher in a
single process is consistently referred to with the switch
--single-process-tests rather than previous content::
specific --single_process in both code and documentation.

Performed basic cleanup to ensure that unused declarations
were removed and made sure switches in content:: are more
consistently declared in the appropriate headers.

Bug: 460513
Change-Id: Ic901d7af972ad43c9e0e2d31e34cc778d388d34a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848365
Reviewed-by: Mike Pinkerton <pinkerton@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705635}
  • Loading branch information
Thomas Lukaszewicz authored and Commit Bot committed Oct 14, 2019
1 parent 2d3d8f0 commit 2c5fb61
Show file tree
Hide file tree
Showing 18 changed files with 57 additions and 58 deletions.
5 changes: 3 additions & 2 deletions apps/load_and_launch_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/process/launch.h"
#include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/test_switches.h"
#include "base/test/test_timeouts.h"
#include "build/branding_buildflags.h"
#include "build/build_config.h"
Expand Down Expand Up @@ -65,7 +66,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
new_cmdline.AppendSwitchNative(apps::kLoadAndLaunchApp,
app_path.value());

new_cmdline.AppendSwitch(content::kLaunchAsBrowser);
new_cmdline.AppendSwitch(switches::kLaunchAsBrowser);
base::Process process =
base::LaunchProcess(new_cmdline, base::LaunchOptionsForTest());
ASSERT_TRUE(process.IsValid());
Expand Down Expand Up @@ -105,7 +106,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,

new_cmdline.AppendSwitchNative(apps::kLoadAndLaunchApp,
app_path.value());
new_cmdline.AppendSwitch(content::kLaunchAsBrowser);
new_cmdline.AppendSwitch(switches::kLaunchAsBrowser);
new_cmdline.AppendArgPath(test_file_path);

base::Process process =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class BitmapFetcherBrowserTest : public InProcessBrowserTest {
}
};

// WARNING: These tests work with --single_process, but not
// WARNING: These tests work with --single-process-tests, but not
// --single-process. The reason is that the sandbox does not get created
// for us by the test process if --single-process is used.

Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ui/send_mouse_move_uitest_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ class SendMouseMoveUITest : public InProcessBrowserTest {
// This test positions the mouse at every point on the screen. It is not meant
// to be run on the bots, as it takes too long. Run it manually as needed to
// verify ui_controls::SendMouseMoveNotifyWhenDone with:
// interactive_ui_tests.exe --single_process --gtest_also_run_disabled_tests \
// --gtest_filter=SendMouseMoveUITest.DISABLED_Fullscreen
// interactive_ui_tests.exe --single-process-tests \
// --gtest_also_run_disabled_tests \
// --gtest_filter=SendMouseMoveUITest.DISABLED_Fullscreen
IN_PROC_BROWSER_TEST_F(SendMouseMoveUITest, DISABLED_Fullscreen) {
// Make the browser fullscreen so that we can position the mouse anywhere on
// the display, as ui_controls::SendMouseMoveNotifyWhenDone can only provide
Expand Down
3 changes: 2 additions & 1 deletion chrome/test/base/chrome_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "base/run_loop.h"
#include "base/strings/string_util.h"
#include "base/test/test_file_util.h"
#include "base/test/test_switches.h"
#include "chrome/app/chrome_main_delegate.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_switches.h"
Expand Down Expand Up @@ -210,7 +211,7 @@ int LaunchChromeTests(size_t parallel_jobs,
// mimics the behavior in standalone Chrome, where this is done in
// chrome/app/chrome_main.cc, which does not get called by tests.
std::unique_ptr<MainThreadStackSamplingProfiler> sampling_profiler;
if (command_line.HasSwitch(content::kLaunchAsBrowser))
if (command_line.HasSwitch(switches::kLaunchAsBrowser))
sampling_profiler = std::make_unique<MainThreadStackSamplingProfiler>();

#if defined(OS_LINUX) || defined(OS_ANDROID)
Expand Down
5 changes: 3 additions & 2 deletions chrome/test/base/in_process_browser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "base/single_thread_task_runner.h"
#include "base/strings/string_number_conversions.h"
#include "base/test/test_file_util.h"
#include "base/test/test_switches.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "chrome/browser/after_startup_task_utils.h"
Expand Down Expand Up @@ -475,9 +476,9 @@ base::CommandLine InProcessBrowserTest::GetCommandLineForRelaunch() {
base::CommandLine::SwitchMap switches =
base::CommandLine::ForCurrentProcess()->GetSwitches();
switches.erase(switches::kUserDataDir);
switches.erase(content::kSingleProcessTestsFlag);
switches.erase(switches::kSingleProcessTests);
switches.erase(switches::kSingleProcess);
new_command_line.AppendSwitch(content::kLaunchAsBrowser);
new_command_line.AppendSwitch(switches::kLaunchAsBrowser);

base::FilePath user_data_dir;
base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
Expand Down
3 changes: 2 additions & 1 deletion chrome/test/base/web_ui_browser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/path_service.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/test_switches.h"
#include "base/threading/thread_restrictions.h"
#include "base/values.h"
#include "chrome/browser/chrome_content_browser_client.h"
Expand Down Expand Up @@ -256,7 +257,7 @@ void BaseWebUIBrowserTest::PreLoadJavascriptLibraries(
libraries_preloaded_ = true;

bool should_wait_flag = base::CommandLine::ForCurrentProcess()->HasSwitch(
::content::kWaitForDebuggerWebUI);
::switches::kWaitForDebuggerWebUI);

if (should_wait_flag)
RunJavascriptUsingHandler("setWaitUser", {}, false, false, preload_host);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ void BrowserPolicyConnectorBase::Shutdown() {
g_testing_provider->Shutdown();
for (const auto& provider : policy_providers_)
provider->Shutdown();
// Drop g_testing_provider so that tests executed with --single_process can
// call SetPolicyProviderForTesting() again. It is still owned by the test.
// Drop g_testing_provider so that tests executed with --single-process-tests
// can call SetPolicyProviderForTesting() again. It is still owned by the
// test.
g_testing_provider = nullptr;
g_created_policy_service = false;
}
Expand Down
10 changes: 8 additions & 2 deletions content/public/common/content_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ const char kV8CacheOptions[] = "v8-cache-options";
// is only useful for tests.
const char kEnableServiceBinaryLauncher[] = "enable-service-binary-launcher";

// Enables the Skia benchmarking extension
// Enables the Skia benchmarking extension.
const char kEnableSkiaBenchmarking[] = "enable-skia-benchmarking";

// On platforms that support it, enables smooth scroll animation.
Expand Down Expand Up @@ -558,9 +558,12 @@ const char kDisableJavaScriptHarmonyShipping[] =
// Enables experimental Harmony (ECMAScript 6) features.
const char kJavaScriptHarmony[] = "javascript-harmony";

// Specifies the flags passed to JS engine
// Specifies the flags passed to JS engine.
const char kJavaScriptFlags[] = "js-flags";

// Flag to launch tests in the browser process.
const char kLaunchAsBrowser[] = "as-browser";

// Overrides the Lite Page Subresource host.
const char kLitePagesServerSubresourceHost[] =
"litepage-server-subresource-host";
Expand Down Expand Up @@ -857,6 +860,9 @@ const char kValidateInputEventStream[] = "validate-input-event-stream";
// kWaitForDebugger flag passed on or not.
const char kWaitForDebuggerChildren[] = "wait-for-debugger-children";

// Flag used by WebUI test runners to wait for debugger to be attached.
const char kWaitForDebuggerWebUI[] = "wait-for-debugger-webui";

// Set the antialiasing method used for webgl. (none, explicit, implicit, or
// screenspace)
const char kWebglAntialiasingMode[] = "webgl-antialiasing-mode";
Expand Down
2 changes: 2 additions & 0 deletions content/public/common/content_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ CONTENT_EXPORT extern const char kIPCConnectionTimeout[];
CONTENT_EXPORT extern const char kIsolateOrigins[];
CONTENT_EXPORT extern const char kJavaScriptFlags[];
CONTENT_EXPORT extern const char kJavaScriptHarmony[];
CONTENT_EXPORT extern const char kLaunchAsBrowser[];
CONTENT_EXPORT extern const char kLitePagesServerSubresourceHost[];
CONTENT_EXPORT extern const char kLogGpuControlListDecisions[];
CONTENT_EXPORT extern const char kLoggingLevel[];
Expand Down Expand Up @@ -248,6 +249,7 @@ CONTENT_EXPORT extern const char kEnableWebRtcSrtpEncryptedHeaders[];
CONTENT_EXPORT extern const char kEnableWebRtcStunOrigin[];
CONTENT_EXPORT extern const char kEnforceWebRtcIPPermissionCheck[];
CONTENT_EXPORT extern const char kForceWebRtcIPHandlingPolicy[];
CONTENT_EXPORT extern const char kWaitForDebuggerWebUI[];
extern const char kWebRtcMaxCaptureFramerate[];
extern const char kWebRtcMaxCpuConsumptionPercentage[];
CONTENT_EXPORT extern const char kWebRtcStunProbeTrialParameter[];
Expand Down
5 changes: 3 additions & 2 deletions content/public/test/browser_test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/synchronization/waitable_event.h"
#include "base/task/post_task.h"
#include "base/test/test_switches.h"
#include "base/test/test_timeouts.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/values.h"
Expand Down Expand Up @@ -1767,8 +1768,8 @@ bool ExecuteWebUIResourceTest(WebContents* web_contents,

DOMMessageQueue message_queue;

bool should_wait_flag =
base::CommandLine::ForCurrentProcess()->HasSwitch(kWaitForDebuggerWebUI);
bool should_wait_flag = base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kWaitForDebuggerWebUI);

if (should_wait_flag) {
ExecuteScriptAsync(
Expand Down
27 changes: 10 additions & 17 deletions content/public/test/test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ base::CommandLine WrapperTestLauncherDelegate::GetCommandLine(
// tests unless this flag was specified to the browser test executable.
new_cmd_line.AppendSwitch("gtest_also_run_disabled_tests");
new_cmd_line.AppendSwitchASCII("gtest_filter", test_name);
new_cmd_line.AppendSwitch(kSingleProcessTestsFlag);
new_cmd_line.AppendSwitch(switches::kSingleProcessTests);
return new_cmd_line;
}

Expand Down Expand Up @@ -243,14 +243,6 @@ void WrapperTestLauncherDelegate::ProcessTestResults(

} // namespace

const char kHelpFlag[] = "help";

const char kLaunchAsBrowser[] = "as-browser";

const char kSingleProcessTestsFlag[] = "single_process";

const char kWaitForDebuggerWebUI[] = "wait-for-debugger-webui";

void AppendCommandLineSwitches() {
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();

Expand All @@ -273,7 +265,7 @@ int LaunchTests(TestLauncherDelegate* launcher_delegate,
const base::CommandLine* command_line =
base::CommandLine::ForCurrentProcess();

if (command_line->HasSwitch(kHelpFlag)) {
if (command_line->HasSwitch(switches::kHelpFlag)) {
PrintUsage();
return 0;
}
Expand Down Expand Up @@ -310,12 +302,12 @@ int LaunchTests(TestLauncherDelegate* launcher_delegate,
// This needs to be before trying to run tests as otherwise utility processes
// end up being launched as a test, which leads to rerunning the test.
if (command_line->HasSwitch(switches::kProcessType) ||
command_line->HasSwitch(kLaunchAsBrowser)) {
command_line->HasSwitch(switches::kLaunchAsBrowser)) {
return ContentMain(params);
}
#endif

if (command_line->HasSwitch(kSingleProcessTestsFlag) ||
if (command_line->HasSwitch(switches::kSingleProcessTests) ||
(command_line->HasSwitch(switches::kSingleProcess) &&
command_line->HasSwitch(base::kGTestFilterFlag)) ||
command_line->HasSwitch(base::kGTestListTestsFlag) ||
Expand All @@ -331,11 +323,12 @@ int LaunchTests(TestLauncherDelegate* launcher_delegate,
TestTimeouts::Initialize();

fprintf(stdout,
"IMPORTANT DEBUGGING NOTE: each test is run inside its own process.\n"
"For debugging a test inside a debugger, use the\n"
"--gtest_filter=<your_test_name> flag along with either\n"
"--single_process (to run the test in one launcher/browser process) or\n"
"--single-process (to do the above, and also run Chrome in single-"
"IMPORTANT DEBUGGING NOTE: each test is run inside its own process.\n"
"For debugging a test inside a debugger, use the\n"
"--gtest_filter=<your_test_name> flag along with either\n"
"--single-process-tests (to run the test in one launcher/browser "
"process) or\n"
"--single-process (to do the above, and also run Chrome in single-"
"process mode).\n");

base::debug::VerifyDebugger();
Expand Down
17 changes: 3 additions & 14 deletions content/public/test/test_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,6 @@ namespace content {
class ContentMainDelegate;
struct ContentMainParams;

extern const char kEmptyTestName[];
extern const char kHelpFlag[];
extern const char kLaunchAsBrowser[];
extern const char kSingleProcessTestsFlag[];

// Flag that causes only the kEmptyTestName test to be run.
extern const char kWarmupFlag[];

// Flag used by WebUI test runners to wait for debugger to be attached.
extern const char kWaitForDebuggerWebUI[];

class TestLauncherDelegate {
public:
virtual int RunTestSuite(int argc, char** argv) = 0;
Expand All @@ -46,13 +35,13 @@ class TestLauncherDelegate {

// Called prior to running each test.
//
// NOTE: this is not called if --single_process is supplied.
// NOTE: this is not called if --single-process-tests is supplied.
virtual void PreRunTest() {}

// Called after running each test. Can modify test result.
//
// NOTE: Just like PreRunTest, this is not called when --single_process is
// supplied.
// NOTE: Just like PreRunTest, this is not called when --single-process-tests
// is supplied.
virtual void PostRunTest(base::TestResult* result) {}

// Allows a TestLauncherDelegate to do work before the launcher shards test
Expand Down
3 changes: 2 additions & 1 deletion content/renderer/render_thread_impl_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/task/post_task.h"
#include "base/task/thread_pool/thread_pool_instance.h"
#include "base/test/test_switches.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/threading/thread_task_runner_handle.h"
#include "content/app/mojo/mojo_init.h"
Expand Down Expand Up @@ -227,7 +228,7 @@ class RenderThreadImplBrowserTest : public testing::Test {

void TearDown() override {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
kSingleProcessTestsFlag)) {
switches::kSingleProcessTests)) {
// In a single-process mode, we need to avoid destructing mock_process_
// because it will call _exit(0) and kill the process before the browser
// side is ready to exit.
Expand Down
11 changes: 6 additions & 5 deletions content/test/content_browser_test_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/test/launcher/test_launcher.h"
#include "base/test/launcher/test_launcher_test_utils.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_switches.h"
#include "base/test/test_timeouts.h"
#include "base/threading/thread_restrictions.h"
#include "base/threading/thread_task_runner_handle.h"
Expand Down Expand Up @@ -82,7 +83,7 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTest, RendererCrashCallStack) {
new_test.AppendSwitchASCII(base::kGTestFilterFlag,
"ContentBrowserTest.MANUAL_RendererCrash");
new_test.AppendSwitch(switches::kRunManualTestsFlag);
new_test.AppendSwitch(kSingleProcessTestsFlag);
new_test.AppendSwitch(switches::kSingleProcessTests);

#if defined(THREAD_SANITIZER)
// TSan appears to not be able to report intentional crashes from sandboxed
Expand Down Expand Up @@ -123,7 +124,7 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTest, BrowserCrashCallStack) {
new_test.AppendSwitchASCII(base::kGTestFilterFlag,
"ContentBrowserTest.MANUAL_BrowserCrash");
new_test.AppendSwitch(switches::kRunManualTestsFlag);
new_test.AppendSwitch(kSingleProcessTestsFlag);
new_test.AppendSwitch(switches::kSingleProcessTests);
std::string output;
base::GetAppOutputAndError(new_test, &output);

Expand Down Expand Up @@ -193,11 +194,11 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTest, RunMockTests) {
// will need to change accordingly.
std::string file_name = "../../content/test/content_browser_test_test.cc";
EXPECT_TRUE(base::test_launcher_utils::ValidateTestLocation(
val, "MockContentBrowserTest.DISABLED_PassTest", file_name, 152));
val, "MockContentBrowserTest.DISABLED_PassTest", file_name, 153));
EXPECT_TRUE(base::test_launcher_utils::ValidateTestLocation(
val, "MockContentBrowserTest.DISABLED_FailTest", file_name, 156));
val, "MockContentBrowserTest.DISABLED_FailTest", file_name, 157));
EXPECT_TRUE(base::test_launcher_utils::ValidateTestLocation(
val, "MockContentBrowserTest.DISABLED_CrashTest", file_name, 160));
val, "MockContentBrowserTest.DISABLED_CrashTest", file_name, 161));

val = root->FindListKey("per_iteration_data");
ASSERT_TRUE(val);
Expand Down
4 changes: 2 additions & 2 deletions docs/linux_debugging.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,10 @@ By default the `browser_tests` forks a new browser for each test. To debug the
browser side of a single test, use a command like

```
gdb --args out/Debug/browser_tests --single_process --gtest_filter=MyTestName
gdb --args out/Debug/browser_tests --single-process-tests --gtest_filter=MyTestName
```

**note the underscore in `single_process`** -- this makes the test harness and
**note the use of `single-process-tests`** -- this makes the test harness and
browser process share the outermost process.


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ public class NativeBrowserTest {
// Set the command line flags to be passed to the C++ main() method. Each
// browser tests Activity should ensure these are included.
public static final String BROWSER_TESTS_FLAGS[] = {
// content::kSingleProcessTestsFlag
"--single_process",
// switches::kSingleProcessTests
"--single-process-tests",

// switches::kUseFakeDeviceForMediaStream
"--use-fake-device-for-media-stream"};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def cmd_line(self, per_test_args):
cmd.append(self._port._path_to_driver())
cmd.append('--gtest_filter=PrintPreviewPdfGeneratedBrowserTest.MANUAL_LayoutTestDriver')
cmd.append('--run-manual')
cmd.append('--single_process')
cmd.append('--single-process-tests')
cmd.extend(per_test_args)
cmd.extend(self._port.get_option('additional_driver_flag', []))
return cmd
Expand Down
2 changes: 1 addition & 1 deletion tools/vscode/launch.json5
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"targetArchitecture": "x64",
"program": "${workspaceRoot}/out/Debug/unit_tests",
"args": ["--gtest_filter=*",
"--single_process",
"--single-process-tests",
"--ui-test-action-max-timeout=1000000",
"--test-launcher-timeout=1000000"],
"preLaunchTask": "5-build_test_debug",
Expand Down

0 comments on commit 2c5fb61

Please sign in to comment.