GTTF: Make the new test launcher exit early if many tests are broken.

Also tag the JSON results indicating this was an early exit.

BUG=236893
R=sky@chromium.org

Review URL: https://codereview.chromium.org/53353005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@232208 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 110 insertions, 65 deletions

diff --git a/base/test/launcher/test_launcher.cc b/base/test/launcher/test_launcher.cc
index 1b2e79a..9ad002b 100644
--- a/base/test/launcher/test_launcher.cc
+++ b/base/test/launcher/test_launcher.cc
@@ -62,56 +62,60 @@ void ShutdownPipeSignalHandler(int signal) {
   HANDLE_EINTR(write(g_shutdown_pipe[1], "q", 1));
 }
 
-// I/O watcher for the reading end of the self-pipe above.
-// Terminates any launched child processes and exits the process.
-class SignalFDWatcher : public MessageLoopForIO::Watcher {
- public:
-  SignalFDWatcher() {
+void KillSpawnedTestProcesses() {
+  // Keep the lock until exiting the process to prevent further processes
+  // from being spawned.
+  AutoLock lock(g_live_process_handles_lock.Get());
+
+  fprintf(stdout,
+          "Sending SIGTERM to %" PRIuS " child processes... ",
+          g_live_process_handles.Get().size());
+  fflush(stdout);
+
+  for (std::set<ProcessHandle>::iterator i =
+           g_live_process_handles.Get().begin();
+       i != g_live_process_handles.Get().end();
+       ++i) {
+    kill((-1) * (*i), SIGTERM);  // Send the signal to entire process group.
   }
 
-  virtual void OnFileCanReadWithoutBlocking(int fd) OVERRIDE {
-    fprintf(stdout, "\nCaught signal. Killing spawned test processes...\n");
-    fflush(stdout);
+  fprintf(stdout,
+          "done.\nGiving processes a chance to terminate cleanly... ");
+  fflush(stdout);
 
-    // Keep the lock until exiting the process to prevent further processes
-    // from being spawned.
-    AutoLock lock(g_live_process_handles_lock.Get());
+  PlatformThread::Sleep(TimeDelta::FromMilliseconds(500));
 
-    fprintf(stdout,
-            "Sending SIGTERM to %" PRIuS " child processes... ",
-            g_live_process_handles.Get().size());
-    fflush(stdout);
+  fprintf(stdout, "done.\n");
+  fflush(stdout);
 
-    for (std::set<ProcessHandle>::iterator i =
-             g_live_process_handles.Get().begin();
-         i != g_live_process_handles.Get().end();
-         ++i) {
-      kill((-1) * (*i), SIGTERM);  // Send the signal to entire process group.
-    }
+  fprintf(stdout,
+          "Sending SIGKILL to %" PRIuS " child processes... ",
+          g_live_process_handles.Get().size());
+  fflush(stdout);
 
-    fprintf(stdout,
-            "done.\nGiving processes a chance to terminate cleanly... ");
-    fflush(stdout);
+  for (std::set<ProcessHandle>::iterator i =
+           g_live_process_handles.Get().begin();
+       i != g_live_process_handles.Get().end();
+       ++i) {
+    kill((-1) * (*i), SIGKILL);  // Send the signal to entire process group.
+  }
 
-    PlatformThread::Sleep(TimeDelta::FromMilliseconds(500));
+  fprintf(stdout, "done.\n");
+  fflush(stdout);
+}
 
-    fprintf(stdout, "done.\n");
-    fflush(stdout);
+// I/O watcher for the reading end of the self-pipe above.
+// Terminates any launched child processes and exits the process.
+class SignalFDWatcher : public MessageLoopForIO::Watcher {
+ public:
+  SignalFDWatcher() {
+  }
 
-    fprintf(stdout,
-            "Sending SIGKILL to %" PRIuS " child processes... ",
-            g_live_process_handles.Get().size());
+  virtual void OnFileCanReadWithoutBlocking(int fd) OVERRIDE {
+    fprintf(stdout, "\nCaught signal. Killing spawned test processes...\n");
     fflush(stdout);
 
-    for (std::set<ProcessHandle>::iterator i =
-             g_live_process_handles.Get().begin();
-         i != g_live_process_handles.Get().end();
-         ++i) {
-      kill((-1) * (*i), SIGKILL);  // Send the signal to entire process group.
-    }
-
-    fprintf(stdout, "done.\n");
-    fflush(stdout);
+    KillSpawnedTestProcesses();
 
     // The signal would normally kill the process, so exit now.
     exit(1);
@@ -210,6 +214,7 @@ TestLauncher::TestLauncher(TestLauncherDelegate* launcher_delegate)
       test_started_count_(0),
       test_finished_count_(0),
       test_success_count_(0),
+      test_broken_count_(0),
       retry_count_(0),
       retry_limit_(3),  // TODO(phajdan.jr): Make a flag control this.
       run_result_(true) {
@@ -254,14 +259,7 @@ bool TestLauncher::Run(int argc, char** argv) {
   if (cycles_ != 1)
     results_tracker_.PrintSummaryOfAllIterations();
 
-  const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kTestLauncherSummaryOutput)) {
-    FilePath summary_path(command_line->GetSwitchValuePath(
-                              switches::kTestLauncherSummaryOutput));
-    if (!results_tracker_.SaveSummaryAsJSON(summary_path)) {
-      LOG(ERROR) << "Failed to save test launcher output summary.";
-    }
-  }
+  MaybeSaveSummaryAsJSON();
 
   return run_result_;
 }
@@ -384,6 +382,30 @@ void TestLauncher::OnTestFinished(const TestResult& result) {
           Bind(&TestLauncher::RunTestIteration, Unretained(this)));
     }
   }
+
+  // Do not waste time on timeouts. We include tests with unknown results here
+  // because sometimes (e.g. hang in between unit tests) that's how a timeout
+  // gets reported.
+  if (result.status == TestResult::TEST_TIMEOUT ||
+      result.status == TestResult::TEST_UNKNOWN) {
+    test_broken_count_++;
+  }
+  size_t broken_threshold =
+      std::max(static_cast<size_t>(10), test_started_count_ / 10);
+  if (test_broken_count_ >= broken_threshold) {
+    fprintf(stdout, "Too many badly broken tests (%" PRIuS "), exiting now.\n",
+            test_broken_count_);
+    fflush(stdout);
+
+#if defined(OS_POSIX)
+    KillSpawnedTestProcesses();
+#endif  // defined(OS_POSIX)
+
+    results_tracker_.AddGlobalTag("BROKEN_TEST_EARLY_EXIT");
+    MaybeSaveSummaryAsJSON();
+
+    exit(1);
+  }
 }
 
 bool TestLauncher::Init() {
@@ -505,6 +527,7 @@ void TestLauncher::RunTestIteration() {
   test_started_count_ = 0;
   test_finished_count_ = 0;
   test_success_count_ = 0;
+  test_broken_count_ = 0;
   retry_count_ = 0;
   tests_to_retry_.clear();
   results_tracker_.OnTestIterationStarting();
@@ -517,6 +540,17 @@ void TestLauncher::RunTestIteration() {
 void TestLauncher::OnAllTestsStarted() {
 }
 
+void TestLauncher::MaybeSaveSummaryAsJSON() {
+  const CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kTestLauncherSummaryOutput)) {
+    FilePath summary_path(command_line->GetSwitchValuePath(
+                              switches::kTestLauncherSummaryOutput));
+    if (!results_tracker_.SaveSummaryAsJSON(summary_path)) {
+      LOG(ERROR) << "Failed to save test launcher output summary.";
+    }
+  }
+}
+
 std::string GetTestOutputSnippet(const TestResult& result,
                                  const std::string& full_output) {
   size_t run_pos = full_output.find(std::string("[ RUN      ] ") +
diff --git a/base/test/launcher/test_launcher.h b/base/test/launcher/test_launcher.h
index eee9851..177d12d 100644
--- a/base/test/launcher/test_launcher.h
+++ b/base/test/launcher/test_launcher.h
@@ -99,6 +99,9 @@ class TestLauncher {
 
   void OnAllTestsStarted();
 
+  // Saves test results summary as JSON if requested from command line.
+  void MaybeSaveSummaryAsJSON();
+
   TestLauncherDelegate* launcher_delegate_;
 
   // Support for outer sharding, just like gtest does.
@@ -120,6 +123,10 @@ class TestLauncher {
   // Number of tests successfully finished in this iteration.
   size_t test_success_count_;
 
+  // Number of tests either timing out or having an unknown result,
+  // likely indicating a more systemic problem if widespread.
+  size_t test_broken_count_;
+
   // Number of retries in this iteration.
   size_t retry_count_;
 
diff --git a/base/test/launcher/test_results_tracker.cc b/base/test/launcher/test_results_tracker.cc
index 895f88b..0760dff 100644
--- a/base/test/launcher/test_results_tracker.cc
+++ b/base/test/launcher/test_results_tracker.cc
@@ -228,9 +228,22 @@ void TestResultsTracker::PrintSummaryOfAllIterations() const {
   fflush(stdout);
 }
 
+void TestResultsTracker::AddGlobalTag(const std::string& tag) {
+  global_tags_.insert(tag);
+}
+
 bool TestResultsTracker::SaveSummaryAsJSON(const FilePath& path) const {
   scoped_ptr<DictionaryValue> summary_root(new DictionaryValue);
 
+  ListValue* global_tags = new ListValue;
+  summary_root->Set("global_tags", global_tags);
+
+  for (std::set<std::string>::const_iterator i = global_tags_.begin();
+       i != global_tags_.end();
+       ++i) {
+    global_tags->AppendString(*i);
+  }
+
   ListValue* per_iteration_data = new ListValue;
   summary_root->Set("per_iteration_data", per_iteration_data);
 
diff --git a/base/test/launcher/test_results_tracker.h b/base/test/launcher/test_results_tracker.h
index 5743507..e129599 100644
--- a/base/test/launcher/test_results_tracker.h
+++ b/base/test/launcher/test_results_tracker.h
@@ -6,6 +6,7 @@
 #define BASE_TEST_LAUNCHER_TEST_RESULTS_TRACKER_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -49,6 +50,10 @@ class TestResultsTracker {
   // Prints a summary of all test iterations (not just the last one) to stdout.
   void PrintSummaryOfAllIterations() const;
 
+  // Adds a string tag to the JSON summary. This is intended to indicate
+  // conditions that affect the entire test run, as opposed to individual tests.
+  void AddGlobalTag(const std::string& tag);
+
   // Saves a JSON summary of all test iterations results to |path|. Returns
   // true on success.
   bool SaveSummaryAsJSON(const FilePath& path) const WARN_UNUSED_RESULT;
@@ -72,6 +77,10 @@ class TestResultsTracker {
 
   ThreadChecker thread_checker_;
 
+  // Set of global tags, i.e. strings indicating conditions that apply to
+  // the entire test run.
+  std::set<std::string> global_tags_;
+
   // Store test results for each iteration.
   std::vector<PerIterationData> per_iteration_data_;
 
diff --git a/content/public/test/test_launcher.cc b/content/public/test/test_launcher.cc
index 97b2343..e70c15f 100644
--- a/content/public/test/test_launcher.cc
+++ b/content/public/test/test_launcher.cc
@@ -90,8 +90,6 @@ class WrapperTestLauncherDelegate : public base::TestLauncherDelegate {
   WrapperTestLauncherDelegate(content::TestLauncherDelegate* launcher_delegate,
                               size_t jobs)
       : launcher_delegate_(launcher_delegate),
-        timeout_count_(0),
-        printed_timeout_message_(false),
         parallel_launcher_(jobs) {
     CHECK(temp_dir_.CreateUniqueTempDir());
   }
@@ -130,13 +128,6 @@ class WrapperTestLauncherDelegate : public base::TestLauncherDelegate {
 
   content::TestLauncherDelegate* launcher_delegate_;
 
-  // Number of times a test timeout occurred.
-  size_t timeout_count_;
-
-  // True after a message about too many timeouts has been printed,
-  // to avoid doing it more than once.
-  bool printed_timeout_message_;
-
   base::ParallelTestLauncher parallel_launcher_;
 
   // Store dependent test name (map is indexed by full test name).
@@ -191,15 +182,6 @@ bool WrapperTestLauncherDelegate::ShouldRunTest(
     return false;
   }
 
-  // Stop test execution after too many timeouts.
-  if (timeout_count_ > 5) {
-    if (!printed_timeout_message_) {
-      printed_timeout_message_ = true;
-      printf("Too many timeouts, aborting test\n");
-    }
-    return false;
-  }
-
   return true;
 }