Launchpad itself

Merge lp:~lifeless/launchpad/zope.testing into lp:launchpad

  1. zope.testing
  2. Merge into devel
Proposed by Robert Collins
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 11814
Proposed branch: lp:~lifeless/launchpad/zope.testing
Merge into: lp:launchpad
Diff against target: 144 lines (+4/-48)
2 files modified
lib/canonical/testing/layers.py (+0/-45)
versions.cfg (+4/-3)
To merge this branch: bzr merge lp:~lifeless/launchpad/zope.testing
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Review via email: mp+39423@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :
Download full text (8.6 KiB)

On Wed, Oct 27, 2010 at 12:36 AM, Robert Collins
<email address hidden> wrote:
> 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:
>

Hey Rob,

I'd be much more comfortable if the change to zope.testing had tests,
even the crappy doctests that zope.testing uses. As far as I can tell
it looks correct, but that's not worth a lot. It's also unfortunate
that the layer tearDown logic is duplicated.

I'm not sure how much to insist on these changes, since our aim is to
move away from layers and zope.testing completely. If we fail in our
aims, or if we get delayed, then we will probably regret not testing
this code.

The changes to Launchpad itself look good.

jml

> 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...

Read more...

Revision history for this message
Jonathan Lange (jml) wrote :

See email

review: Needs Fixing
Revision history for this message
Robert Collins (lifeless) wrote :

jml has approved in person.

Revision history for this message
Robert Collins (lifeless) wrote :

garh, but still needs the vote >< sigh ec2land.

Revision history for this message
Jonathan Lange (jml) wrote :

This patch increases the cost of maintaining our zope.testing fork, but probably not substantially, and better than the certain risk of building on crap.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/testing/layers.py'
2--- lib/canonical/testing/layers.py 2010-10-27 01:23:01 +0000
3+++ lib/canonical/testing/layers.py 2010-10-27 04:36:06 +0000
4@@ -51,7 +51,6 @@
5 'reconnect_stores',
6 ]
7
8-import atexit
9 import datetime
10 import errno
11 import gc
12@@ -573,11 +572,6 @@
13 pidfile = MemcachedLayer.getPidFile()
14 open(pidfile, 'w').write(str(MemcachedLayer._memcached_process.pid))
15
16- # Register an atexit hook just in case tearDown doesn't get
17- # invoked for some perculiar reason.
18- if not BaseLayer.persist_test_services:
19- atexit.register(kill_by_pidfile, pidfile)
20-
21 @classmethod
22 @profiled
23 def tearDown(cls):
24@@ -623,8 +617,6 @@
25
26 _is_setup = False
27
28- _atexit_call = None
29-
30 @classmethod
31 @profiled
32 def setUp(cls):
33@@ -637,7 +629,6 @@
34 the_librarian = LibrarianTestSetup()
35 the_librarian.setUp()
36 LibrarianLayer._check_and_reset()
37- cls._atexit_call = atexit.register(the_librarian.tearDown)
38
39 @classmethod
40 @profiled
41@@ -652,11 +643,6 @@
42 )
43 LibrarianLayer._check_and_reset()
44 LibrarianTestSetup().tearDown()
45- # Remove the atexit handler, since we've already done the work.
46- atexit._exithandlers = [
47- handler for handler in atexit._exithandlers
48- if handler[0] != cls._atexit_call]
49- cls._atexit_call = None
50
51 @classmethod
52 @profiled
53@@ -908,24 +894,6 @@
54 pass
55
56 @classmethod
57- def tearDownHelper(cls):
58- """Helper for when LaunchpadLayer is mixed with unteardownable layers.
59-
60- E.g. FunctionalLayer causes other layer tearDown to not occur, which is
61- why atexit is used, but because test runners delegate rather than
62- returning, the librarian and other servers are only killed *at the end
63- of the whole test run*, which leads to multiple instances running, so
64- we manually run the teardown for these layers.
65- """
66- try:
67- MemcachedLayer.tearDown()
68- finally:
69- try:
70- LibrarianLayer.tearDown()
71- finally:
72- DatabaseLayer.tearDown()
73-
74- @classmethod
75 @profiled
76 def testSetUp(cls):
77 # By default, don't make external service tests timeout.
78@@ -1224,7 +1192,6 @@
79 def setUp(cls):
80 google = GoogleServiceTestSetup()
81 google.setUp()
82- atexit.register(google.tearDown)
83
84 @classmethod
85 def tearDown(cls):
86@@ -1284,11 +1251,6 @@
87
88 @classmethod
89 @profiled
90- def tearDown(cls):
91- LaunchpadLayer.tearDownHelper()
92-
93- @classmethod
94- @profiled
95 def testSetUp(cls):
96 # Reset any statistics
97 from canonical.launchpad.webapp.opstats import OpStats
98@@ -1395,7 +1357,6 @@
99 def tearDown(cls):
100 if not globalregistry.base.unregisterUtility(cls._mailbox):
101 raise NotImplementedError('failed to unregister mailbox')
102- LaunchpadLayer.tearDownHelper()
103
104 @classmethod
105 @profiled
106@@ -1716,9 +1677,6 @@
107 log.propagate = False
108 cls.smtp_controller = SMTPController('localhost', 9025)
109 cls.smtp_controller.start()
110- # Make sure that the smtp server is killed even if tearDown() is
111- # skipped, which can happen if FunctionalLayer is in the mix.
112- atexit.register(cls.stopSMTPServer)
113
114 @classmethod
115 @profiled
116@@ -1729,9 +1687,6 @@
117 cls._cleanUpStaleAppServer()
118 cls._runAppServer()
119 cls._waitUntilAppServerIsReady()
120- # Make sure that the app server is killed even if tearDown() is
121- # skipped.
122- atexit.register(cls.stopAppServer)
123
124 @classmethod
125 @profiled
126
127=== modified file 'versions.cfg'
128--- versions.cfg 2010-10-23 01:59:54 +0000
129+++ versions.cfg 2010-10-27 04:36:06 +0000
130@@ -227,10 +227,11 @@
131 zope.tal = 3.5.1
132 zope.tales = 3.4.0
133 zope.testbrowser = 3.7.0a1
134-# Build of lp:~mars/zope.testing/3.9.4-p1. Fixes bugs 570380 and 587886.
135-# With patch for thread leaks to make them skips, fixes windmill errors with
136+# p1 Build of lp:~mars/zope.testing/3.9.4-p1. Fixes bugs 570380 and 587886.
137+# p2 With patch for thread leaks to make them skips, fixes windmill errors with
138 # 'new threads' in hudson/ec2 builds.
139-zope.testing = 3.9.4-p2
140+# p3 And always tear down layers, because thats the Right Thing To Do.
141+zope.testing = 3.9.4-p3
142 zope.thread = 3.4
143 zope.traversing = 3.8.0
144 zope.viewlet = 3.6.1