← Back to team overview

testtools-dev team mailing list archive

[Merge] lp:~jml/testtools/trial-hang into lp:testtools

 

Jonathan Lange has proposed merging lp:~jml/testtools/trial-hang into lp:testtools.

Requested reviews:
  testtools committers (testtools-committers)
Related bugs:
  Bug #926189 in testtools: "Using the Trial test runner results in a spinning test"
  https://bugs.launchpad.net/testtools/+bug/926189

For more details, see:
https://code.launchpad.net/~jml/testtools/trial-hang/+merge/91470

Turns out that running testtools Twisted tests can cause hangs in Trial.  That really sucks.

Looking into it, it was caused by a logic bug in run_with_log_observers.  It stored 'real_observers', which it treated as a snapshot of the Twisted log observers at the start of the method.  We use it to restore the observers when our function is done.

However, 'real_observers' was just a reference to the maintained list of current observers.  That meant that when we added a log observer, the list of real_observers expanded, which made the 'for observer in real_observers' loop keep expanding. 

The fix is very simple, as is the test.
-- 
https://code.launchpad.net/~jml/testtools/trial-hang/+merge/91470
Your team testtools developers is subscribed to branch lp:testtools.
=== modified file 'testtools/deferredruntest.py'
--- testtools/deferredruntest.py	2011-07-20 16:39:27 +0000
+++ testtools/deferredruntest.py	2012-02-03 18:04:24 +0000
@@ -57,7 +57,7 @@
 
 def run_with_log_observers(observers, function, *args, **kwargs):
     """Run 'function' with the given Twisted log observers."""
-    real_observers = log.theLogPublisher.observers
+    real_observers = list(log.theLogPublisher.observers)
     for observer in real_observers:
         log.theLogPublisher.removeObserver(observer)
     for observer in observers:

=== modified file 'testtools/tests/test_deferredruntest.py'
--- testtools/tests/test_deferredruntest.py	2011-07-27 09:39:08 +0000
+++ testtools/tests/test_deferredruntest.py	2012-02-03 18:04:24 +0000
@@ -13,6 +13,7 @@
 from testtools.content import (
     text_content,
     )
+from testtools.deferredruntest import run_with_log_observers
 from testtools.helpers import try_import
 from testtools.matchers import (
     Equals,
@@ -746,6 +747,18 @@
             lambda x: self.fail("Should not have succeeded"), check_result)
 
 
+class TestRunWithLogObservers(TestCase):
+
+    def test_restores_observers(self):
+        from twisted.python import log
+        # Make sure there's at least one observer.  This reproduces bug
+        # #926189.
+        log.addObserver(lambda *args: None)
+        observers = list(log.theLogPublisher.observers)
+        run_with_log_observers([], lambda: None)
+        self.assertEqual(observers, log.theLogPublisher.observers)
+
+
 def test_suite():
     from unittest import TestLoader, TestSuite
     return TestSuite(


Follow ups