Thread Watcher: don't fail when test already failed.

testing_support/thread_watcher.py

Index: testing_support/thread_watcher.py
diff --git a/testing_support/thread_watcher.py b/testing_support/thread_watcher.py
index ca5180bad59f101c4da41700586ec3b6663d86b5..5584f8125e30947c1afd36e86d44ebf51d1f4812 100644
--- a/testing_support/thread_watcher.py
+++ b/testing_support/thread_watcher.py
@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 
+import logging
 import sys
 import threading
 import traceback
@@ -30,9 +31,29 @@ class ThreadWatcherMixIn(object):
           details.append('  Thread stopped while acquiring stacktrace.\n')
       details = ''.join(details)
 
-      self.fail('Found %d running thread(s) after the test.\n%s' % (
-        len(new_threads), details))
+      msg = 'Found %d running thread(s) after the test.\n%s' % (
+          len(new_threads), details)
+      if self._should_fail():
+        self.fail(msg)  # This raises exception.
+      # Test failed before this check, so don't raise another exception

[Sergiy Byelozyorov, 2016/07/07 07:05:01]

IMHO this comment should be before line 36 and IMHO we shouldn't  print warnings
below as user will see thread errors after fixing their code anyway.

[tandrii(chromium), 2016/07/07 11:47:05]

Done.

+      # overwriting the original one, and making it harder to debug.
+      # Besides, that exception might be the reason cleanup didn't happen.
+      logging.warn(msg)
+      logging.warn('Note: extra running threads might be due to your test '
+                   'failure, which prevented *your cleanup code* from '
+                   'executing.')
 
+  def _should_fail(self):

[Sergiy Byelozyorov, 2016/07/07 07:05:01]

suggestion: rename to _test_likely_passed(self):

[tandrii(chromium), 2016/07/07 11:47:05]

Done.

+    """Conservatively returns True if current test has likely not failed yet."""

[Sergiy Byelozyorov, 2016/07/07 07:05:01]

How about this?

"""Conservatively returns True if current test has likely not failed yet.

This is necessary to determine whether we should fail the test when finding
stray threads. If the test has failed in some other way, we should avoid
throwing an exception because it will mask the original error."""

[tandrii(chromium), 2016/07/07 11:47:05]

Done.

+    # When run using unittest runner,
+    # self._resultForDoCleanups is set to internal unittest object.
+    if hasattr(self, '_resultForDoCleanups') and self._resultForDoCleanups:
+      return self._resultForDoCleanups.wasSuccessful()
+    # expect_tests runner does so differently.
+    if hasattr(self, '_test_failed_with_exception'):
+      return not self._test_failed_with_exception
+    # Default case - be conservative.
+    return True
 
 
 class TestCase(unittest.TestCase, ThreadWatcherMixIn):