Merge lp:~jml/launchpad/failing-tests-bug-711209 into lp:launchpad

Proposed by Jonathan Lange
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 12294
Proposed branch: lp:~jml/launchpad/failing-tests-bug-711209
Merge into: lp:launchpad
Diff against target: 81 lines (+15/-11)
2 files modified
lib/canonical/testing/layers.py (+15/-4)
lib/lp/testing/__init__.py (+0/-7)
To merge this branch: bzr merge lp:~jml/launchpad/failing-tests-bug-711209
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+48184@code.launchpad.net

Commit message

[r=sinzui][ui=none][bug=711209] Twisted tests no longer fail silently

Description of the change

This branch address bug 711209. Some Twisted tests in our suite were not failing when they ought to have been failing. This was because of a band-aid to solve a very similar problem, that turned out to be self-defeating once we were running tests without the underlying wound. i.e. the bandaid helps Trial tests, but not testtools Twisted tests.

The branch "solves" this problem by moving the bandaid into the TwistedLayer, which is only used for Trial tests. It has the pleasant side-effect of removing an import side-effect.

I verified that the fix works by adding a failing test to test_builder.py and watching it fail.

Note that fixing this bug might expose other tests that have been failing silently. Best thing to do is try to land this then watch the failures that come back from the server.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you for providing this. I learn something about zope's frame introspection too.

review: Approve (code)

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 2011-01-28 19:28:40 +0000
3+++ lib/canonical/testing/layers.py 2011-02-01 16:03:56 +0000
4@@ -56,7 +56,6 @@
5 import gc
6 import logging
7 import os
8-import shutil
9 import signal
10 import socket
11 import subprocess
12@@ -69,7 +68,10 @@
13 from unittest import TestCase, TestResult
14 from urllib import urlopen
15
16-from fixtures import Fixture
17+from fixtures import (
18+ Fixture,
19+ MonkeyPatch,
20+ )
21 import psycopg2
22 from storm.zope.interfaces import IZStorm
23 import transaction
24@@ -595,8 +597,8 @@
25 time.sleep(0.1)
26
27 # Store the pidfile for other processes to kill.
28- pidfile = MemcachedLayer.getPidFile()
29- open(pidfile, 'w').write(str(MemcachedLayer._memcached_process.pid))
30+ pid_file = MemcachedLayer.getPidFile()
31+ open(pid_file, 'w').write(str(MemcachedLayer._memcached_process.pid))
32
33 @classmethod
34 @profiled
35@@ -1219,6 +1221,14 @@
36 TwistedLayer._save_signals()
37 from twisted.internet import interfaces, reactor
38 from twisted.python import threadpool
39+ # zope.exception demands more of frame objects than
40+ # twisted.python.failure provides in its fake frames. This is enough
41+ # to make it work with them as of 2009-09-16. See
42+ # https://bugs.launchpad.net/bugs/425113.
43+ cls._patch = MonkeyPatch(
44+ 'twisted.python.failure._Frame.f_locals',
45+ property(lambda self: {}))
46+ cls._patch.setUp()
47 if interfaces.IReactorThreads.providedBy(reactor):
48 pool = getattr(reactor, 'threadpool', None)
49 # If the Twisted threadpool has been obliterated (probably by
50@@ -1240,6 +1250,7 @@
51 if pool is not None:
52 reactor.threadpool.stop()
53 reactor.threadpool = None
54+ cls._patch.cleanUp()
55 TwistedLayer._restore_signals()
56
57
58
59=== modified file 'lib/lp/testing/__init__.py'
60--- lib/lp/testing/__init__.py 2011-01-17 22:31:30 +0000
61+++ lib/lp/testing/__init__.py 2011-02-01 16:03:56 +0000
62@@ -87,10 +87,6 @@
63 from testtools.content import Content
64 from testtools.content_type import UTF8_TEXT
65 import transaction
66-# zope.exception demands more of frame objects than twisted.python.failure
67-# provides in its fake frames. This is enough to make it work with them
68-# as of 2009-09-16. See https://bugs.launchpad.net/bugs/425113.
69-from twisted.python.failure import _Frame
70 from windmill.authoring import WindmillTestClient
71 from zope.component import (
72 adapter,
73@@ -163,9 +159,6 @@
74 from lp.testing.matchers import Provides
75
76
77-_Frame.f_locals = property(lambda self: {})
78-
79-
80 class FakeTime:
81 """Provides a controllable implementation of time.time().
82