launchpad-reviewers team mailing list archive

[Free Ekanayaka <free.ekanayaka@xxxxxxxxxxxxx>]

Free Ekanayaka has proposed merging lp:~free.ekanayaka/txlongpoll/fast-rabbit-reset into lp:txlongpoll.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~free.ekanayaka/txlongpoll/fast-rabbit-reset/+merge/278287

This branch adds a few improvements to the test suite:

- Take advantage of testresources.OptimisingTestSuite in order to avoid a wholesale shutdown/restart of the rabbit process at each test and just delete the queues created by the test instead.

- Silence the Twisted logger in FrontEndAjaxTest.test_render_error.

- Remove the workaround of aborting tearDown early without running any asynchronous logic (see the comment in the code).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~free.ekanayaka/txlongpoll/fast-rabbit-reset into lp:txlongpoll.

=== modified file 'txlongpoll/testing/__init__.py'
--- txlongpoll/testing/__init__.py	2011-06-28 16:21:03 +0000
+++ txlongpoll/testing/__init__.py	2015-11-23 09:10:16 +0000
@@ -1,2 +1,10 @@
 # Copyright 2005-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
+from testtools import run
+from testresources import OptimisingTestSuite
+
+# It seems there's no way to indicate the desired suite class to use via the
+# command line, so we set it in the module directly. Since this is the only
+# tweak we need, it's not worth creating a custom run() entry point and
+# instantiating a test runner/loader by hand.
+run.defaultTestLoaderCls.suiteClass = OptimisingTestSuite

=== modified file 'txlongpoll/testing/client.py'
--- txlongpoll/testing/client.py	2011-09-30 09:56:12 +0000
+++ txlongpoll/testing/client.py	2015-11-23 09:10:16 +0000
@@ -36,12 +36,22 @@
         return self._real_queue_get(timeout)
 
 
+class RabbitServerWithoutReset(RabbitServer):
+
+    def reset(self):
+        """No-op reset.
+
+        Logic to cleanup relevant state is delegated to the test case, which is
+        in charge to delete the queues it created (see AMQTest.tearDown).
+        """
+
+
 class AMQTest(ResourcedTestCase, TestCase):
 
     run_tests_with = AsynchronousDeferredRunTestForBrokenTwisted.make_factory(
         timeout=5)
 
-    resources = [('rabbit', FixtureResource(RabbitServer()))]
+    resources = [('rabbit', FixtureResource(RabbitServerWithoutReset()))]
 
     VHOST = "/"
     USER = "guest"
@@ -65,18 +75,23 @@
             self.factory)
         return self.connected_deferred
 
+    def tearDown(self):
+        # The AsynchronousDeferredRunTest machinery from testools doesn't play
+        # well with an asynchronous tearDown decorated with inlineCallbacks,
+        # because testtools.TestCase._run_teardown attempts to ensure that the
+        # test class upcalls testtools.TestCase.tearDown and raises an error if
+        # it doesn't find it. To workaround that, we move the inlineCallbacks
+        # logic in a separate method, so we can run the superclass tearDown()
+        # before returning.
+        deferred = self._deleteQueuesAndExchanges()
+        super(AMQTest, self).tearDown()
+        return deferred
+
     @inlineCallbacks
-    def tearDown(self):
-        # XXX: Moving this up here to silence a nigh-on inexplicable error
-        # that occurs when it's at the bottom of the function.
+    def _deleteQueuesAndExchanges(self):
+        """Delete any queue or exchange that the test might have created."""
         self.factory.stopTrying()
         self.connector.disconnect()
-        super(AMQTest, self).tearDown()
-
-        # XXX: This is only safe because we tear down the whole server.
-        #      We can't run this after the tearDown above, because the
-        #      fixture is gone.
-        return
 
         self.connected_deferred = Deferred()
         factory = AMQFactory(self.USER, self.PASSWORD, self.VHOST,

=== modified file 'txlongpoll/tests/test_frontend.py'
--- txlongpoll/tests/test_frontend.py	2011-09-30 09:56:12 +0000
+++ txlongpoll/tests/test_frontend.py	2015-11-23 09:10:16 +0000
@@ -6,7 +6,10 @@
 from unittest import defaultTestLoader
 
 from testtools import TestCase
-from testtools.deferredruntest import assert_fails_with
+from testtools.deferredruntest import (
+    assert_fails_with,
+    run_with_log_observers,
+    )
 from twisted.internet import reactor
 from twisted.internet.defer import (
     Deferred,
@@ -374,7 +377,7 @@
         """
         self.message_queue.messages["uuid1"] = ValueError("Not there")
         request = FakeRequest({"uuid": ["uuid1"], "sequence": ["0"]})
-        self.ajax.render(request)
+        run_with_log_observers([], self.ajax.render, request)
         self.assertEquals(request.written.getvalue(), "Not there")
         self.assertEquals(request.code, 500)