Thread Watcher: don't fail when test already failed.
This prevents hiding the likely real root cause of the failure.
Does not depend, but works with https://codereview.chromium.org/2121343004
change to expect_tests.
R=machenbach@chromium.org, sergiyb@chromium.org
BUG=
Review URL: https://codereview.chromium.org/2127943002 .
diff --git a/testing_support/tests/thread_watcher_test.py b/testing_support/tests/thread_watcher_test.py
index 4989f2d..c98be29 100644
--- a/testing_support/tests/thread_watcher_test.py
+++ b/testing_support/tests/thread_watcher_test.py
@@ -28,6 +28,13 @@
self._stop_event.set()
self.join()
+class _ResultFake(object):
+ def __init__(self, value):
+ self.value = value
+
+ def wasSuccessful(self):
+ return self.value
+
class ThreadWatcherTestCase(thread_watcher.TestCase):
def setUp(self):
@@ -71,6 +78,38 @@
self.assertNotIn(' Thread stopped while acquiring stacktrace.\n',
error_message)
+ def test_extra_threads_unittest_pass(self):
+ # Test succeeded, so thread_watcher must cause failure.
+ self.watcher._resultForDoCleanups = _ResultFake(True)
+ self.watcher.setUp()
+ self._threads.append(_PuppetThread('foo'))
+ self.watcher.tearDown()
+ self.assertEqual(len(self._fail_called), 1)
+
+ def test_extra_threads_unittest_fail(self):
+ # Test failed already, so thread_watcher must ignore result.
+ self.watcher._resultForDoCleanups = _ResultFake(False)
+ self.watcher.setUp()
+ self._threads.append(_PuppetThread('foo'))
+ self.watcher.tearDown()
+ self.assertEqual(self._fail_called, [])
+
+ def test_extra_threads_expect_tests_pass(self):
+ # Test succeeded, so thread_watcher must cause failure.
+ self.watcher._test_failed_with_exception = False
+ self.watcher.setUp()
+ self._threads.append(_PuppetThread('foo'))
+ self.watcher.tearDown()
+ self.assertEqual(len(self._fail_called), 1)
+
+ def test_extra_threads_expect_tests_fail(self):
+ # Test failed already, so thread_watcher must ignore result.
+ self.watcher._test_failed_with_exception = True
+ self.watcher.setUp()
+ self._threads.append(_PuppetThread('foo'))
+ self.watcher.tearDown()
+ self.assertEqual(self._fail_called, [])
+
def test_fail_get_stacktrace(self):
self.watcher.setUp()
self._threads.append(_PuppetThread('foo'))
@@ -108,6 +147,5 @@
self._threads[-1].stop()
-
if __name__ == '__main__':
unittest.main()
diff --git a/testing_support/thread_watcher.py b/testing_support/thread_watcher.py
index ca5180b..85246f3 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
@@ -14,6 +15,12 @@
self._pre_test_threads = [t.ident for t in threading.enumerate()]
def tearDown(self):
+ # If test failed before this check, don't raise another exception
+ # overwriting the original one, and making it harder to debug.
+ # Besides, that exception might be the reason cleanup didn't happen.
+ if self._test_definitely_failed():
+ return
+
post_test_threads = threading.enumerate()
new_threads = [t for t in post_test_threads
if t.ident not in self._pre_test_threads]
@@ -31,8 +38,25 @@
details = ''.join(details)
self.fail('Found %d running thread(s) after the test.\n%s' % (
- len(new_threads), details))
+ len(new_threads), details))
+ def _test_definitely_failed(self):
+ """Returns True only if test has definitely failed already.
+
+ 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.
+ """
+ # When run using unittest runner,
+ # self._resultForDoCleanups is set to internal unittest object.
+ if hasattr(self, '_resultForDoCleanups') and self._resultForDoCleanups:
+ return not self._resultForDoCleanups.wasSuccessful()
+ # expect_tests runner does so differently (see
+ # https://codereview.chromium.org/2121343004).
+ if hasattr(self, '_test_failed_with_exception'):
+ return self._test_failed_with_exception
+ # Default case - be conservative.
+ return False
class TestCase(unittest.TestCase, ThreadWatcherMixIn):
