launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/zope.testing into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Fix layers to teardown always not just sometimes, fixing teardown in our code base and freeing us from the tyranny of atexit.

This is the diff to zope.testing:

diff -rup zope.testing-3.9.4-p2/setup.py zope.testing-3.9.4-p3/setup.py
--- zope.testing-3.9.4-p2/setup.py	2010-10-21 13:14:02.000000000 +1100
+++ zope.testing-3.9.4-p3/setup.py	2010-10-27 14:07:33.574856366 +1100
@@ -85,7 +85,7 @@ long_description=(
 
 setup(
     name='zope.testing',
-    version = '3.9.4-p2',
+    version = '3.9.4-p3',
     url='http://pypi.python.org/pypi/zope.testing',
     license='ZPL 2.1',
     description='Zope testing framework, including the testrunner script.',
Only in zope.testing-3.9.4-p3: setup.py~
diff -rup zope.testing-3.9.4-p2/src/zope/testing/testrunner/runner.py zope.testing-3.9.4-p3/src/zope/testing/testrunner/runner.py
--- zope.testing-3.9.4-p2/src/zope/testing/testrunner/runner.py	2010-06-08 02:05:10.000000000 +1000
+++ zope.testing-3.9.4-p3/src/zope/testing/testrunner/runner.py	2010-10-27 15:09:31.956110181 +1100
@@ -606,6 +606,22 @@ def tear_down_unneeded(options, needed, 
         except NotImplementedError:
             output.tear_down_not_supported()
             if not optional:
+                # Unwind all layers without mangling setup_layers - the runner
+                # will start a new process.
+                unneeded = [togo for togo in setup_layers if togo is not l]
+                unneeded = order_by_bases(unneeded)
+                unneeded.reverse()
+                for to_clean in unneeded:
+                    output.start_tear_down(name_from_layer(to_clean))
+                    t = time.time()
+                    tear_down = getattr(to_clean, 'tearDown', None)
+                    if tear_down:
+                        try:
+                            tear_down()
+                        except NotImplementedError:
+                            output.tear_down_not_supported()
+                        else:
+                            output.stop_tear_down(time.time() - t)
                 raise CanNotTearDown(l)
         else:
             output.stop_tear_down(time.time() - t)
Only in zope.testing-3.9.4-p3/src/zope/testing/testrunner: runner.py~
diff -rup zope.testing-3.9.4-p2/src/zope.testing.egg-info/PKG-INFO zope.testing-3.9.4-p3/src/zope.testing.egg-info/PKG-INFO
--- zope.testing-3.9.4-p2/src/zope.testing.egg-info/PKG-INFO	2010-10-21 13:14:07.000000000 +1100
+++ zope.testing-3.9.4-p3/src/zope.testing.egg-info/PKG-INFO	2010-10-27 15:09:36.178570346 +1100
@@ -1,6 +1,6 @@
 Metadata-Version: 1.0
 Name: zope.testing
-Version: 3.9.4-p2
+Version: 3.9.4-p3
 Summary: Zope testing framework, including the testrunner script.
 Home-page: http://pypi.python.org/pypi/zope.testing
 Author: Zope Foundation and Contributors

-- 
https://code.launchpad.net/~lifeless/launchpad/zope.testing/+merge/39423
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/zope.testing into lp:launchpad/devel.

=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2010-10-27 01:23:01 +0000
+++ lib/canonical/testing/layers.py	2010-10-27 04:36:06 +0000
@@ -51,7 +51,6 @@
     'reconnect_stores',
     ]
 
-import atexit
 import datetime
 import errno
 import gc
@@ -573,11 +572,6 @@
         pidfile = MemcachedLayer.getPidFile()
         open(pidfile, 'w').write(str(MemcachedLayer._memcached_process.pid))
 
-        # Register an atexit hook just in case tearDown doesn't get
-        # invoked for some perculiar reason.
-        if not BaseLayer.persist_test_services:
-            atexit.register(kill_by_pidfile, pidfile)
-
     @classmethod
     @profiled
     def tearDown(cls):
@@ -623,8 +617,6 @@
 
     _is_setup = False
 
-    _atexit_call = None
-
     @classmethod
     @profiled
     def setUp(cls):
@@ -637,7 +629,6 @@
         the_librarian = LibrarianTestSetup()
         the_librarian.setUp()
         LibrarianLayer._check_and_reset()
-        cls._atexit_call = atexit.register(the_librarian.tearDown)
 
     @classmethod
     @profiled
@@ -652,11 +643,6 @@
                     )
         LibrarianLayer._check_and_reset()
         LibrarianTestSetup().tearDown()
-        # Remove the atexit handler, since we've already done the work.
-        atexit._exithandlers = [
-            handler for handler in atexit._exithandlers
-            if handler[0] != cls._atexit_call]
-        cls._atexit_call = None
 
     @classmethod
     @profiled
@@ -908,24 +894,6 @@
         pass
     
     @classmethod
-    def tearDownHelper(cls):
-        """Helper for when LaunchpadLayer is mixed with unteardownable layers.
-
-        E.g. FunctionalLayer causes other layer tearDown to not occur, which is
-        why atexit is used, but because test runners delegate rather than
-        returning, the librarian and other servers are only killed *at the end
-        of the whole test run*, which leads to multiple instances running, so
-        we manually run the teardown for these layers.
-        """
-        try:
-            MemcachedLayer.tearDown()
-        finally:
-            try:
-                LibrarianLayer.tearDown()
-            finally:
-                DatabaseLayer.tearDown()
-
-    @classmethod
     @profiled
     def testSetUp(cls):
         # By default, don't make external service tests timeout.
@@ -1224,7 +1192,6 @@
     def setUp(cls):
         google = GoogleServiceTestSetup()
         google.setUp()
-        atexit.register(google.tearDown)
 
     @classmethod
     def tearDown(cls):
@@ -1284,11 +1251,6 @@
 
     @classmethod
     @profiled
-    def tearDown(cls):
-        LaunchpadLayer.tearDownHelper()
-
-    @classmethod
-    @profiled
     def testSetUp(cls):
         # Reset any statistics
         from canonical.launchpad.webapp.opstats import OpStats
@@ -1395,7 +1357,6 @@
     def tearDown(cls):
         if not globalregistry.base.unregisterUtility(cls._mailbox):
             raise NotImplementedError('failed to unregister mailbox')
-        LaunchpadLayer.tearDownHelper()
 
     @classmethod
     @profiled
@@ -1716,9 +1677,6 @@
         log.propagate = False
         cls.smtp_controller = SMTPController('localhost', 9025)
         cls.smtp_controller.start()
-        # Make sure that the smtp server is killed even if tearDown() is
-        # skipped, which can happen if FunctionalLayer is in the mix.
-        atexit.register(cls.stopSMTPServer)
 
     @classmethod
     @profiled
@@ -1729,9 +1687,6 @@
         cls._cleanUpStaleAppServer()
         cls._runAppServer()
         cls._waitUntilAppServerIsReady()
-        # Make sure that the app server is killed even if tearDown() is
-        # skipped.
-        atexit.register(cls.stopAppServer)
 
     @classmethod
     @profiled

=== modified file 'versions.cfg'
--- versions.cfg	2010-10-23 01:59:54 +0000
+++ versions.cfg	2010-10-27 04:36:06 +0000
@@ -227,10 +227,11 @@
 zope.tal = 3.5.1
 zope.tales = 3.4.0
 zope.testbrowser = 3.7.0a1
-# Build of lp:~mars/zope.testing/3.9.4-p1.  Fixes bugs 570380 and 587886.
-# With patch for thread leaks to make them skips, fixes windmill errors with
+# p1 Build of lp:~mars/zope.testing/3.9.4-p1.  Fixes bugs 570380 and 587886.
+# p2 With patch for thread leaks to make them skips, fixes windmill errors with
 # 'new threads' in hudson/ec2 builds.
-zope.testing = 3.9.4-p2
+# p3 And always tear down layers, because thats the Right Thing To Do.
+zope.testing = 3.9.4-p3
 zope.thread = 3.4
 zope.traversing = 3.8.0
 zope.viewlet = 3.6.1