Check for stray threads from tests and fail if found

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
new file mode 100644
index 0000000000000000000000000000000000000000..6929ffc38e6006ecc10c30b84fe111f072101ede
--- /dev/null
+++ b/testing_support/thread_watcher.py
@@ -0,0 +1,31 @@
+# Copyright (c) 2011 The Chromium Authors. All rights reserved.

[tandrii(chromium), 2016/06/13 17:03:28]

nit 2016

[Sergiy Byelozyorov, 2016/06/14 06:12:14]

Done.

+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+
+import threading
+import unittest
+
+
+class ThreadWatcherMixIn(object):
+  def setUp(self):
+    self._pre_test_threads = [t.ident for t in threading.enumerate()]

[tandrii(chromium), 2016/06/13 17:03:28]

strictly speaking, these numbers can be re-used.

See https://docs.python.org/3/library/threading.html#threading.Thread.ident

So, the code is below is correct when the failure is reported, but it may
actually loose some cases.

[Sergiy Byelozyorov, 2016/06/14 06:12:14]

Agree, but I am not sure how otherwise we can check it. All threads that existed
before test should belong to the test launcher and shouldn't stop due to the
actions of the test. I can perhaps add thread name to the identification, but I
doubt it will make things much better.

+
+  def tearDown(self):
+    new_threads = [t for t in threading.enumerate()
+                   if t.ident not in self._pre_test_threads]
+    if new_threads:
+      self.fail('Found %d running thread(s) after the test: %s' % (
+        len(new_threads), ', '.join(t.name for t in new_threads)))

[tandrii(chromium), 2016/06/13 17:03:28]

t.name is usually not given, at least in infra land. Thus, this line is not
helpful. It's better to report a stack trace for each thread.

[Sergiy Byelozyorov, 2016/06/14 06:12:14]

t.name defaults to Thread-NN, where NN-some unique number. It's just as good as
thread.ident. Regarding stacktraces - I'm not quite sure how to do them in
Python. In most places where I looked, I see suggestions to use
sys._current_frames(), but I'm somewhat worried about using private methods from
the standard library. IMHO, from the context of the CL introducing this error
it's likely clear what threads were started and not shut down. Also I've named
all threads in commit_queue (but not elsewhere in infra/infra_internal).
However, if you think using sys._current_frames is okay, then I'll use it. WDYT?

[tandrii(chromium), 2016/06/14 10:21:43]

Yes, and as result both suck for debugging, unless you carefully name every
thread created (like you did for CQ :) ). That's why stack traces are much more
useful.
Well, depends on the CL. Even you did naming of the CQ threads, so I guess it
wasn't clear :)
It's OK since both Py2 and Py3 has it, and we aren't going to move away from
CPython.
I personally use this way: http://stackoverflow.com/a/24334576

[Sergiy Byelozyorov, 2016/06/14 19:47:01]

Done.

+
+
+
+class TestCase(unittest.TestCase, ThreadWatcherMixIn):
+  """Base unittest class that cleans off a trial directory in tearDown()."""
+  def setUp(self):
+    super(TestCase, self).setUp()
+    ThreadWatcherMixIn.setUp(self)
+
+  def tearDown(self):
+    ThreadWatcherMixIn.tearDown(self)
+    super(TestCase, self).tearDown()