(reland) base::LaunchUnitTestsSerially should not be overridable by cmdline

(reland) base::LaunchUnitTestsSerially should ignore the --test-launcher-jobs
command line switch, and never use parallel jobs.  To achieve this, change
callers of base::TestLauncher to pass the requested number of parallel test
jobs, instead of the default number of parallel test jobs.

And while we're at it, let's remove TestLauncherDelegate's
AdjustDefaultParallelJobs method, which no longer has any users.

BUG=735587,736837
TBR=rdevlin.cronin@chromium.org

Change-Id: I4085fb21c1dce467527210407e3913ff3b5e3bc6
Reviewed-on: https://chromium-review.googlesource.com/549342
Commit-Queue: Mostyn Bramley-Moore <mostynb@opera.com>
Reviewed-by: Henrik Kjellander <kjellander@chromium.org>
Reviewed-by: Alok Priyadarshi <alokp@chromium.org>
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483909}

base/test/launcher/test_launcher.cc

@@ -21,6 +21,7 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/process/kill.h"
 #include "base/process/launch.h"
 #include "base/run_loop.h"
@@ -33,6 +34,7 @@
 #include "base/strings/stringize_macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/sys_info.h"
 #include "base/test/gtest_util.h"
 #include "base/test/launcher/test_launcher_tracer.h"
 #include "base/test/launcher/test_results_tracker.h"
@@ -803,22 +805,6 @@ bool TestLauncher::Init() {
   if (command_line->HasSwitch(switches::kTestLauncherForceRunBrokenTests))
     force_run_broken_tests_ = true;
 
-  if (command_line->HasSwitch(switches::kTestLauncherJobs)) {
-    size_t jobs = 0U;
-    if (!StringToSizeT(command_line->GetSwitchValueASCII(
-                         switches::kTestLauncherJobs), &jobs) ||
-        !jobs) {
-      LOG(ERROR) << "Invalid value for " << switches::kTestLauncherJobs;
-      return false;
-    }
-
-    parallel_jobs_ = jobs;
-  } else if (command_line->HasSwitch(kGTestFilterFlag) && !BotModeEnabled()) {
-    // Do not run jobs in parallel by default if we are running a subset of
-    // the tests and if bot mode is off.
-    parallel_jobs_ = 1U;
-  }
-
   fprintf(stdout, "Using %" PRIuS " parallel jobs.\n", parallel_jobs_);
   fflush(stdout);
   if (parallel_jobs_ > 1U) {
@@ -1199,6 +1185,31 @@ scoped_refptr<TaskRunner> TestLauncher::GetTaskRunner() {
   return worker_thread_->task_runner();
 }
 
+size_t NumParallelJobs() {
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  CommandLine::SwitchMap switches = command_line->GetSwitches();
+
+  size_t jobs = 0U;
+
+  if (command_line->HasSwitch(kGTestFilterFlag) && !BotModeEnabled()) {
+    // Do not run jobs in parallel by default if we are running a subset of
+    // the tests and if bot mode is off.
+    return 1U;
+  } else if (command_line->HasSwitch(switches::kTestLauncherJobs)) {
+    if (!StringToSizeT(
+            command_line->GetSwitchValueASCII(switches::kTestLauncherJobs),
+            &jobs) ||
+        !jobs) {
+      LOG(ERROR) << "Invalid value for " << switches::kTestLauncherJobs;
+      return 0U;
+    }
+
+    return jobs;
+  }
+
+  return base::checked_cast<size_t>(SysInfo::NumberOfProcessors());
+}
+
 std::string GetTestOutputSnippet(const TestResult& result,
                                  const std::string& full_output) {
   size_t run_pos = full_output.find(std::string("[ RUN      ] ") +

base/test/launcher/test_launcher.h

@@ -244,6 +244,9 @@ class TestLauncher {
   DISALLOW_COPY_AND_ASSIGN(TestLauncher);
 };
 
+// Return the number of parallel jobs to use, or 0U in case of error.
+size_t NumParallelJobs();
+
 // Extract part from |full_output| that applies to |result|.
 std::string GetTestOutputSnippet(const TestResult& result,
                                  const std::string& full_output);

base/test/launcher/unit_test_launcher.cc

@@ -183,7 +183,7 @@ bool GetSwitchValueAsInt(const std::string& switch_name, int* result) {
 }
 
 int LaunchUnitTestsInternal(const RunTestSuiteCallback& run_test_suite,
-                            int default_jobs,
+                            size_t parallel_jobs,
                             int default_batch_limit,
                             bool use_job_objects,
                             const Closure& gtest_init) {
@@ -246,7 +246,7 @@ int LaunchUnitTestsInternal(const RunTestSuiteCallback& run_test_suite,
   DefaultUnitTestPlatformDelegate platform_delegate;
   UnitTestLauncherDelegate delegate(
       &platform_delegate, batch_limit, use_job_objects);
-  base::TestLauncher launcher(&delegate, default_jobs);
+  base::TestLauncher launcher(&delegate, parallel_jobs);
   bool success = launcher.Run();
 
   fprintf(stdout, "Tests took %" PRId64 " seconds.\n",
@@ -455,40 +455,33 @@ int LaunchUnitTests(int argc,
                     char** argv,
                     const RunTestSuiteCallback& run_test_suite) {
   CommandLine::Init(argc, argv);
-  return LaunchUnitTestsInternal(
+  size_t parallel_jobs = NumParallelJobs();
-      run_test_suite,
+  if (parallel_jobs == 0U) {
-      SysInfo::NumberOfProcessors(),
+    return 1;
-      kDefaultTestBatchLimit,
+  }
-      true,
+  return LaunchUnitTestsInternal(run_test_suite, parallel_jobs,
-      Bind(&InitGoogleTestChar, &argc, argv));
+                                 kDefaultTestBatchLimit, true,
+                                 Bind(&InitGoogleTestChar, &argc, argv));
 }
 
 int LaunchUnitTestsSerially(int argc,
                             char** argv,
                             const RunTestSuiteCallback& run_test_suite) {
   CommandLine::Init(argc, argv);
-  return LaunchUnitTestsInternal(
+  return LaunchUnitTestsInternal(run_test_suite, 1U, kDefaultTestBatchLimit,
-      run_test_suite,
+                                 true, Bind(&InitGoogleTestChar, &argc, argv));
-      1,
-      kDefaultTestBatchLimit,
-      true,
-      Bind(&InitGoogleTestChar, &argc, argv));
 }
 
-int LaunchUnitTestsWithOptions(
+int LaunchUnitTestsWithOptions(int argc,
-    int argc,
+                               char** argv,
-    char** argv,
+                               size_t parallel_jobs,
-    int default_jobs,
+                               int default_batch_limit,
-    int default_batch_limit,
+                               bool use_job_objects,
-    bool use_job_objects,
+                               const RunTestSuiteCallback& run_test_suite) {
-    const RunTestSuiteCallback& run_test_suite) {
   CommandLine::Init(argc, argv);
-  return LaunchUnitTestsInternal(
+  return LaunchUnitTestsInternal(run_test_suite, parallel_jobs,
-      run_test_suite,
+                                 default_batch_limit, use_job_objects,
-      default_jobs,
+                                 Bind(&InitGoogleTestChar, &argc, argv));
-      default_batch_limit,
-      use_job_objects,
-      Bind(&InitGoogleTestChar, &argc, argv));
 }
 
 #if defined(OS_WIN)
@@ -498,12 +491,13 @@ int LaunchUnitTests(int argc,
                     const RunTestSuiteCallback& run_test_suite) {
   // Windows CommandLine::Init ignores argv anyway.
   CommandLine::Init(argc, NULL);
-  return LaunchUnitTestsInternal(
+  size_t parallel_jobs = NumParallelJobs();
-      run_test_suite,
+  if (parallel_jobs == 0U) {
-      SysInfo::NumberOfProcessors(),
+    return 1;
-      kDefaultTestBatchLimit,
+  }
-      use_job_objects,
+  return LaunchUnitTestsInternal(run_test_suite, parallel_jobs,
-      Bind(&InitGoogleTestWChar, &argc, argv));
+                                 kDefaultTestBatchLimit, use_job_objects,
+                                 Bind(&InitGoogleTestWChar, &argc, argv));
 }
 #endif  // defined(OS_WIN)
 

base/test/launcher/unit_test_launcher.h

@@ -29,17 +29,16 @@ int LaunchUnitTestsSerially(int argc,
                             const RunTestSuiteCallback& run_test_suite);
 
 // Launches unit tests in given test suite. Returns exit code.
-// |default_jobs| is the default number of parallel test jobs.
+// |parallel_jobs| is the number of parallel test jobs.
 // |default_batch_limit| is the default size of test batch
 // (use 0 to disable batching).
 // |use_job_objects| determines whether to use job objects.
-int LaunchUnitTestsWithOptions(
+int LaunchUnitTestsWithOptions(int argc,
-    int argc,
+                               char** argv,
-    char** argv,
+                               size_t parallel_jobs,
-    int default_jobs,
+                               int default_batch_limit,
-    int default_batch_limit,
+                               bool use_job_objects,
-    bool use_job_objects,
+                               const RunTestSuiteCallback& run_test_suite);
-    const RunTestSuiteCallback& run_test_suite);
 
 #if defined(OS_WIN)
 // Launches unit tests in given test suite. Returns exit code.

chrome/test/base/browser_perf_tests_main.cc

@@ -8,7 +8,9 @@
 int main(int argc, char** argv) {
   // Always run browser perf tests serially - parallel running would be less
   // deterministic and distort perf measurements.
+  size_t parallel_jobs = 1U;
+
   ChromeTestSuiteRunner runner;
   ChromeTestLauncherDelegate delegate(&runner);
-  return LaunchChromeTests(1, &delegate, argc, argv);
+  return LaunchChromeTests(parallel_jobs, &delegate, argc, argv);
 }

chrome/test/base/browser_tests_main.cc

@@ -2,15 +2,21 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <algorithm>
+#include "base/command_line.h"
-
+#include "base/test/launcher/test_launcher.h"
-#include "base/sys_info.h"
 #include "chrome/test/base/chrome_test_launcher.h"
 #include "chrome/test/base/chrome_test_suite.h"
 
 int main(int argc, char** argv) {
-  int default_jobs = std::max(1, base::SysInfo::NumberOfProcessors() / 2);
+  base::CommandLine::Init(argc, argv);
+  size_t parallel_jobs = base::NumParallelJobs();
+  if (parallel_jobs == 0U) {
+    return 1;
+  } else if (parallel_jobs > 1U) {
+    parallel_jobs /= 2U;
+  }
+
   ChromeTestSuiteRunner runner;
   ChromeTestLauncherDelegate delegate(&runner);
-  return LaunchChromeTests(default_jobs, &delegate, argc, argv);
+  return LaunchChromeTests(parallel_jobs, &delegate, argc, argv);
 }

chrome/test/base/browser_tests_main_chromeos.cc

@@ -2,9 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <algorithm>
+#include "base/command_line.h"
-
+#include "base/test/launcher/test_launcher.h"
-#include "base/sys_info.h"
 #include "chrome/test/base/chrome_test_launcher.h"
 #include "chrome/test/base/chrome_test_suite.h"
 #include "chrome/test/base/mash_browser_tests_main.h"
@@ -14,8 +13,15 @@ int main(int argc, char** argv) {
   if (RunMashBrowserTests(argc, argv, &exit_code))
     return exit_code;
 
-  int default_jobs = std::max(1, base::SysInfo::NumberOfProcessors() / 2);
+  base::CommandLine::Init(argc, argv);
+  size_t parallel_jobs = base::NumParallelJobs();
+  if (parallel_jobs == 0U) {
+    return 1;
+  } else if (parallel_jobs > 1U) {
+    parallel_jobs /= 2U;
+  }
+
   ChromeTestSuiteRunner runner;
   ChromeTestLauncherDelegate delegate(&runner);
-  return LaunchChromeTests(default_jobs, &delegate, argc, argv);
+  return LaunchChromeTests(parallel_jobs, &delegate, argc, argv);
 }

chrome/test/base/chrome_test_launcher.cc

@@ -125,7 +125,7 @@ void ChromeTestLauncherDelegate::PreSharding() {
 #endif
 }
 
-int LaunchChromeTests(int default_jobs,
+int LaunchChromeTests(size_t parallel_jobs,
                       content::TestLauncherDelegate* delegate,
                       int argc,
                       char** argv) {
@@ -146,5 +146,5 @@ int LaunchChromeTests(int default_jobs,
   crash_reporter::SetCrashReporterClient(crash_client);
 #endif
 
-  return content::LaunchTests(delegate, default_jobs, argc, argv);
+  return content::LaunchTests(delegate, parallel_jobs, argc, argv);
 }

chrome/test/base/chrome_test_launcher.h

@@ -44,10 +44,10 @@ class ChromeTestLauncherDelegate : public content::TestLauncherDelegate {
   DISALLOW_COPY_AND_ASSIGN(ChromeTestLauncherDelegate);
 };
 
-// Launches Chrome browser tests. |default_jobs| is number of test jobs to be
+// Launches Chrome browser tests. |parallel_jobs| is number of test jobs to be
-// run in parallel, unless overridden from the command line. Returns exit code.
+// run in parallel. Returns exit code.
 // Does not take ownership of ChromeTestLauncherDelegate.
-int LaunchChromeTests(int default_jobs,
+int LaunchChromeTests(size_t parallel_jobs,
                       content::TestLauncherDelegate* delegate,
                       int argc,
                       char** argv);

chrome/test/base/interactive_ui_tests_main.cc

@@ -102,11 +102,13 @@ int main(int argc, char** argv) {
   InProcessBrowserTest::set_global_browser_set_up_function(
       &ui_test_utils::BringBrowserWindowToFront);
 #endif
+
   // Run interactive_ui_tests serially, they do not support running in parallel.
-  int default_jobs = 1;
+  size_t parallel_jobs = 1U;
+
   InteractiveUITestSuiteRunner runner;
   ChromeTestLauncherDelegate delegate(&runner);
-  const int result = LaunchChromeTests(default_jobs, &delegate, argc, argv);
+  const int result = LaunchChromeTests(parallel_jobs, &delegate, argc, argv);
 #if defined(OS_WIN)
   KillAlwaysOnTopWindows(RunType::AFTER_TEST);
 #endif

chrome/test/base/mash_browser_tests_main.cc

@@ -19,6 +19,7 @@
 #include "base/run_loop.h"
 #include "base/sys_info.h"
 #include "base/task_scheduler/task_scheduler.h"
+#include "base/test/launcher/test_launcher.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "chrome/app/mash/embedded_services.h"
@@ -201,7 +202,10 @@ bool RunMashBrowserTests(int argc, char** argv, int* exit_code) {
     return true;
   }
 
-  int default_jobs = std::max(1, base::SysInfo::NumberOfProcessors() / 2);
+  size_t parallel_jobs = base::NumParallelJobs();
+  if (parallel_jobs > 1U) {
+    parallel_jobs /= 2U;
+  }
   MashTestLauncherDelegate delegate;
   // --single_process and no service pipe token indicate we were run directly
   // from the command line. In this case we have to start up
@@ -214,6 +218,6 @@ bool RunMashBrowserTests(int argc, char** argv, int* exit_code) {
     content::ServiceManagerConnection::SetFactoryForTest(
         &service_manager_connection_factory);
   }
-  *exit_code = LaunchChromeTests(default_jobs, &delegate, argc, argv);
+  *exit_code = LaunchChromeTests(parallel_jobs, &delegate, argc, argv);
   return true;
 }

chromecast/app/cast_test_launcher.cc

@@ -2,8 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/command_line.h"
 #include "base/macros.h"
 #include "base/sys_info.h"
+#include "base/test/launcher/test_launcher.h"
 #include "base/test/test_suite.h"
 #include "chromecast/app/cast_main_delegate.h"
 #include "content/public/test/test_launcher.h"
@@ -41,8 +43,12 @@ class CastTestLauncherDelegate : public content::TestLauncherDelegate {
 }  // namespace chromecast
 
 int main(int argc, char** argv) {
-  int default_jobs = std::max(1, base::SysInfo::NumberOfProcessors() / 2);
+  base::CommandLine::Init(argc, argv);
+  size_t parallel_jobs = base::NumParallelJobs();
+  if (parallel_jobs > 1U) {
+    parallel_jobs /= 2U;
+  }
   chromecast::shell::CastTestLauncherDelegate launcher_delegate;
   mojo::edk::Init();
-  return content::LaunchTests(&launcher_delegate, default_jobs, argc, argv);
+  return content::LaunchTests(&launcher_delegate, parallel_jobs, argc, argv);
 }

content/public/test/test_launcher.cc

@@ -481,7 +481,7 @@ TestLauncherDelegate::~TestLauncherDelegate() {
 }
 
 int LaunchTests(TestLauncherDelegate* launcher_delegate,
-                int default_jobs,
+                size_t parallel_jobs,
                 int argc,
                 char** argv) {
   DCHECK(!g_launcher_delegate);
@@ -546,11 +546,8 @@ int LaunchTests(TestLauncherDelegate* launcher_delegate,
 
   launcher_delegate->PreSharding();
 
-  // Allow the |launcher_delegate| to modify |default_jobs|.
-  launcher_delegate->AdjustDefaultParallelJobs(&default_jobs);
-
   WrapperTestLauncherDelegate delegate(launcher_delegate);
-  base::TestLauncher launcher(&delegate, default_jobs);
+  base::TestLauncher launcher(&delegate, parallel_jobs);
   const int result = launcher.Run() ? 0 : 1;
   launcher_delegate->OnDoneRunningTests();
   return result;

content/public/test/test_launcher.h

@@ -62,10 +62,6 @@ class TestLauncherDelegate {
   // jobs.
   virtual void PreSharding() {}
 
-  // Allows a TestLauncherDelegate to adjust the number of |default_jobs| used
-  // when --test-launcher-jobs isn't specified on the command-line.
-  virtual void AdjustDefaultParallelJobs(int* default_jobs) {}
-
   // Called prior to returning from LaunchTests(). Gives the delegate a chance
   // to do cleanup before state created by TestLauncher has been destroyed (such
   // as the AtExitManager).
@@ -75,11 +71,10 @@ class TestLauncherDelegate {
   virtual ~TestLauncherDelegate();
 };
 
-// Launches tests using |launcher_delegate|. |default_jobs| is number
+// Launches tests using |launcher_delegate|. |parallel_jobs| is the number
-// of test jobs to be run in parallel, unless overridden from the command line.
+// of test jobs to be run in parallel.
-// Returns exit code.
 int LaunchTests(TestLauncherDelegate* launcher_delegate,
-                int default_jobs,
+                size_t parallel_jobs,
                 int argc,
                 char** argv) WARN_UNUSED_RESULT;
 

content/test/content_test_launcher.cc

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/process/memory.h"
 #include "base/sys_info.h"
+#include "base/test/launcher/test_launcher.h"
 #include "base/test/test_suite.h"
 #include "base/test/test_timeouts.h"
 #include "build/build_config.h"
@@ -126,7 +127,11 @@ class ContentTestLauncherDelegate : public TestLauncherDelegate {
 }  // namespace content
 
 int main(int argc, char** argv) {
-  int default_jobs = std::max(1, base::SysInfo::NumberOfProcessors() / 2);
+  base::CommandLine::Init(argc, argv);
+  size_t parallel_jobs = base::NumParallelJobs();
+  if (parallel_jobs > 1U) {
+    parallel_jobs /= 2U;
+  }
   content::ContentTestLauncherDelegate launcher_delegate;
-  return LaunchTests(&launcher_delegate, default_jobs, argc, argv);
+  return LaunchTests(&launcher_delegate, parallel_jobs, argc, argv);
 }

extensions/shell/test/shell_tests_main.cc

@@ -2,14 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <algorithm>
+#include "base/command_line.h"
-
+#include "base/test/launcher/test_launcher.h"
-#include "base/sys_info.h"
 #include "extensions/shell/test/shell_test_launcher_delegate.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 int main(int argc, char** argv) {
-  int default_jobs = std::max(1, base::SysInfo::NumberOfProcessors() / 2);
+  base::CommandLine::Init(argc, argv);
+  size_t parallel_jobs = base::NumParallelJobs();
+  if (parallel_jobs > 1U) {
+    parallel_jobs /= 2U;
+  }
   extensions::AppShellTestLauncherDelegate launcher_delegate;
-  return content::LaunchTests(&launcher_delegate, default_jobs, argc, argv);
+  return content::LaunchTests(&launcher_delegate, parallel_jobs, argc, argv);
 }

headless/test/headless_test_launcher.cc

@@ -5,8 +5,9 @@
 #include <memory>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/macros.h"
-#include "base/sys_info.h"
+#include "base/test/launcher/test_launcher.h"
 #include "content/public/test/content_test_suite_base.h"
 #include "content/public/test/test_launcher.h"
 #include "headless/lib/browser/headless_browser_impl.h"
@@ -63,7 +64,11 @@ class HeadlessTestLauncherDelegate : public content::TestLauncherDelegate {
 }  // namespace headless
 
 int main(int argc, char** argv) {
-  int default_jobs = std::max(1, base::SysInfo::NumberOfProcessors() / 2);
+  base::CommandLine::Init(argc, argv);
+  size_t parallel_jobs = base::NumParallelJobs();
+  if (parallel_jobs > 1U) {
+    parallel_jobs /= 2U;
+  }
   headless::HeadlessTestLauncherDelegate launcher_delegate;
-  return LaunchTests(&launcher_delegate, default_jobs, argc, argv);
+  return LaunchTests(&launcher_delegate, parallel_jobs, argc, argv);
 }