Merge lp:~benji/launchpad/bug-963463 into lp:launchpad

[Benji York]

Fix tests that use twisted and leave DelayedCalls, uncalled in the reactor.

Bug 963463 is about tests failing intermittently when run in groups (in
parallel, but that doesn't matter).

The problem is that there are tests that use twisted and leave
DelayedCalls, uncalled in the reactor. Tests using testtools later
detect the dirty reactor and generates an error.

This branch fixes several (all?) instances of those tests by giving them
a tearDown that clears out the reactor.

Lint: "make lint" reported several "E301 expected 1 blank line, found 0"
in ./lib/lp/registry/tests/test_distributionmirror_prober.py which I
fixed.

QA: since this is a testing bug, there is no QA to do.

[Brad Crittenden (bac)]

Hey Benji this looks great. On IRC I twisted (ha!) into agreeing to add a helper function so those common bits aren't repeated.

Thanks.

review: Approve (code)

=== modified file 'lib/lp/registry/tests/test_distributionmirror_prober.py'
--- lib/lp/registry/tests/test_distributionmirror_prober.py	2012-02-28 05:36:01 +0000
+++ lib/lp/registry/tests/test_distributionmirror_prober.py	2012-04-02 20:59:22 +0000
@@ -69,6 +69,14 @@
     )
 
 
+def clean_up_reactor():
+    # XXX: JonathanLange 2010-11-22: These tests leave stacks of delayed
+    # calls around.  They need to be updated to use Twisted correctly.
+    # For the meantime, just blat the reactor.
+    for delayed_call in reactor.getDelayedCalls():
+        delayed_call.cancel()
+
+
 class HTTPServerTestSetup(TacTestSetup):
 
     def setUpRoot(self):
@@ -142,16 +150,20 @@
         connect to the given host.
         """
         prober = ProberFactory(url)
+
         def fakeConnect(host, port, factory):
             factory.connecting_to = host
             factory.succeeded('200')
+
         prober.connecting_to = None
         orig_connect = reactor.connectTCP
         reactor.connectTCP = fakeConnect
+
         def restore_connect(result, orig_connect):
             self.failUnlessEqual(prober.connecting_to, host)
             reactor.connectTCP = orig_connect
             return None
+
         deferred = prober.probe()
         return deferred.addCallback(restore_connect, orig_connect)
 
@@ -179,11 +191,13 @@
         self.failUnless(prober.redirection_count == 0)
         self.failUnless(prober.url == url)
         deferred = prober.probe()
+
         def got_result(result):
             self.failUnless(prober.redirection_count == 1)
             new_url = 'http://localhost:%s/valid-mirror/file' % self.port
             self.failUnless(prober.url == new_url)
             self.failUnless(result == str(httplib.OK))
+
         return deferred.addCallback(got_result)
 
     def test_redirectawareprober_detects_infinite_loop(self):
@@ -200,26 +214,32 @@
 
     def test_200(self):
         d = self._createProberAndProbe(self.urls['200'])
+
         def got_result(result):
             self.failUnless(
                 result == str(httplib.OK),
                 "Expected a '200' status but got '%s'" % result)
+
         return d.addCallback(got_result)
 
     def test_success_cancel_timeout_call(self):
         prober = ProberFactory(self.urls['200'])
         deferred = prober.probe()
         self.failUnless(prober.timeoutCall.active())
+
         def check_timeout_call(result):
             self.failIf(prober.timeoutCall.active())
+
         return deferred.addCallback(check_timeout_call)
 
     def test_failure_cancel_timeout_call(self):
         prober = ProberFactory(self.urls['500'])
         deferred = prober.probe()
         self.failUnless(prober.timeoutCall.active())
+
         def check_timeout_call(result):
             self.failIf(prober.timeoutCall.active())
+
         return deferred.addErrback(check_timeout_call)
 
     def test_notfound(self):
@@ -299,6 +319,8 @@
         # Restore the globals that our tests fiddle with.
         distributionmirror_prober.host_requests = self.orig_host_requests
         distributionmirror_prober.host_timeouts = self.orig_host_timeouts
+        # We need to remove any DelayedCalls that didn't actually get called.
+        clean_up_reactor()
         super(
             TestProberFactoryRequestTimeoutRatioWithoutTwisted,
             self).tearDown()
@@ -402,22 +424,26 @@
         host = 'localhost'
         d = self._createProberAndProbe(
             u'http://%s:%s/timeout' % (host, self.port))
+
         def got_error(error):
             self.failUnlessEqual(
                 {host: 1}, distributionmirror_prober.host_requests)
             self.failUnlessEqual(
                 {host: 1}, distributionmirror_prober.host_timeouts)
+
         return d.addErrback(got_error)
 
     def test_non_timeout_is_recorded(self):
         host = 'localhost'
         d = self._createProberAndProbe(
             u'http://%s:%s/valid-mirror' % (host, self.port))
+
         def got_result(result):
             self.failUnlessEqual(
                 {host: 1}, distributionmirror_prober.host_requests)
             self.failUnlessEqual(
                 {host: 0}, distributionmirror_prober.host_timeouts)
+
         return d.addCallback(got_result)
 
     def test_failure_after_too_many_timeouts(self):
@@ -518,6 +544,11 @@
 
 class TestRedirectAwareProberFactoryAndProtocol(TestCase):
 
+    def tearDown(self):
+        # We need to remove any DelayedCalls that didn't actually get called.
+        clean_up_reactor()
+        super(TestRedirectAwareProberFactoryAndProtocol, self).tearDown()
+
     def test_redirect_resets_timeout(self):
         prober = RedirectAwareProberFactory('http://foo.bar')
         prober.timeoutCall = FakeTimeOutCall()
@@ -532,16 +563,20 @@
         prober = RedirectAwareProberFactory(url)
         prober.timeoutCall = FakeTimeOutCall()
         prober.connectCalled = False
+
         def connect():
             prober.connectCalled = True
+
         prober.connect = connect
         return prober
 
     def test_raises_error_if_redirected_to_different_file(self):
         prober = self._createFactoryAndStubConnectAndTimeoutCall(
             'http://foo.bar/baz/boo/package.deb')
+
         def failed(error):
             prober.has_failed = True
+
         prober.failed = failed
         prober.redirect('http://foo.bar/baz/boo/notfound?file=package.deb')
         self.failUnless(prober.has_failed)
@@ -622,8 +657,10 @@
     def getLogger(self):
         logger = logging.getLogger('distributionmirror-prober')
         logger.errorCalled = False
+
         def error(msg):
             logger.errorCalled = True
+
         logger.error = error
         return logger
 
@@ -739,13 +776,16 @@
         # not a ProberTimeout, a BadResponseCode or a ConnectionSkipped.
         d = defer.Deferred()
         d.addErrback(callbacks.deleteMirrorSeries)
+        ok = []
+
         def got_result(result):
             self.fail(
                 "Any failure that's not a timeout/bad-response/skipped "
                 "should be propagated.")
-        ok = []
+
         def got_failure(failure):
             ok.append(1)
+
         d.addCallbacks(got_result, got_failure)
         d.errback(Failure(ZeroDivisionError()))
         self.assertEqual([1], ok)
@@ -786,11 +826,8 @@
         RequestManager.host_locks.clear()
 
     def tearDown(self):
-        # XXX: JonathanLange 2010-11-22: These tests leave stacks of delayed
-        # calls around.  They need to be updated to use Twisted correctly.
-        # For the meantime, just blat the reactor.
-        for delayed_call in reactor.getDelayedCalls():
-            delayed_call.cancel()
+        # We need to remove any DelayedCalls that didn't actually get called.
+        clean_up_reactor()
         super(TestProbeFunctionSemaphores, self).tearDown()
 
     def test_MirrorCDImageSeries_records_are_deleted_before_probing(self):
@@ -879,6 +916,11 @@
 
 class TestLoggingMixin(TestCase):
 
+    def tearDown(self):
+        # We need to remove any DelayedCalls that didn't actually get called.
+        clean_up_reactor()
+        super(TestLoggingMixin, self).tearDown()
+
     def _fake_gettime(self):
         # Fake the current time.
         fake_time = datetime(2004, 10, 20, 12, 00, 00, 000000)