launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24133
[Merge] ~cjwatson/launchpad:doctest-capture-oopses into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:doctest-capture-oopses into launchpad:master.
Commit message:
Capture OOPSes in doctests
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/375762
TestCase has captured OOPSes in Launchpad's unit tests since 2010, but we've never systematically done the same thing for doctests. This occasionally causes some problems: with amqp >= 2.4.0 it can mean that attempts to publish OOPSes to a nonexistent exchange leave unhandled errors lying around to trip up later tests (see https://code.launchpad.net/~cjwatson/python-oops-amqp/publisher-handle-channel-errors/+merge/367748), which is extremely confusing and difficult to debug, and I suspect has caused a number of transient failures on buildbot.
We now systematically capture OOPSes for doctests just as we do for unit tests, ensuring better test isolation. I cleaned up a few ad-hoc arrangements in individual doctests.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:doctest-capture-oopses into launchpad:master.
diff --git a/lib/lp/app/stories/basics/xx-request-expired.txt b/lib/lp/app/stories/basics/xx-request-expired.txt
index 2e3306a..fe21067 100644
--- a/lib/lp/app/stories/basics/xx-request-expired.txt
+++ b/lib/lp/app/stories/basics/xx-request-expired.txt
@@ -8,9 +8,6 @@ causing a soft timeout to be logged.
>>> from lp.services.config import config
>>> from textwrap import dedent
- >>> from lp.testing.fixture import CaptureOops
- >>> capture = CaptureOops()
- >>> capture.setUp()
>>> test_data = dedent("""
... [database]
... db_statement_timeout: 1
@@ -28,11 +25,9 @@ causing a soft timeout to be logged.
<title>Error: Timeout</title>
...
- >>> capture.oopses[0]['type']
+ >>> oops_capture.oopses[-1]['type']
'RequestExpired'
- >>> capture.cleanUp()
-
Let's reset the config value we changed:
>>> base_test_data = config.pop('base_test_data')
diff --git a/lib/lp/app/stories/basics/xx-soft-timeout.txt b/lib/lp/app/stories/basics/xx-soft-timeout.txt
index 65fa191..9a2e175 100644
--- a/lib/lp/app/stories/basics/xx-soft-timeout.txt
+++ b/lib/lp/app/stories/basics/xx-soft-timeout.txt
@@ -39,9 +39,6 @@ If we set soft_request_timeout to some value, the page will take
slightly longer then the soft_request_timeout value to generate, thus
causing a soft timeout to be logged.
- >>> from lp.testing.fixture import CaptureOops
- >>> capture = CaptureOops()
- >>> capture.setUp()
>>> from textwrap import dedent
>>> test_data = (dedent("""
... [database]
@@ -57,13 +54,9 @@ causing a soft timeout to be logged.
...
Soft timeout threshold is set to 1 ms. This page took ... ms to render.
- >>> capture.oopses[0]['type']
+ >>> oops_capture.oopses[-1]['type']
'SoftRequestTimeout'
-Unregister from the oops collection:
-
- >>> capture.cleanUp()
-
Since the page rendered correctly, we assume it's a soft timeout error,
since otherwise we would have gotten an OOPS page when we tried to
render the page.
diff --git a/lib/lp/bugs/doc/externalbugtracker.txt b/lib/lp/bugs/doc/externalbugtracker.txt
index 2a902f4..1e3a99a 100644
--- a/lib/lp/bugs/doc/externalbugtracker.txt
+++ b/lib/lp/bugs/doc/externalbugtracker.txt
@@ -750,17 +750,13 @@ We temporarily silence the logging from this function because we're not
interested in it. Again, the watch's lastchecked field will also be
updated.
- >>> from lp.testing.fixture import CaptureOops
- >>> capture = CaptureOops()
- >>> capture.setUp()
-
>>> external_bugtracker.initialize_remote_bugdb_error = None
>>> for error in [UnparsableBugData, Exception]:
... example_bugwatch.lastchecked = None
... external_bugtracker.get_remote_status_error = error
... bug_watch_updater.updateBugWatches(
... external_bugtracker, [example_bugwatch])
- ... oops = capture.oopses[-1]
+ ... oops = oops_capture.oopses[-1]
... print("%s: %s (%s; %s)" % (
... example_bugwatch.last_error_type.title,
... example_bugwatch.lastchecked is not None,
@@ -768,8 +764,6 @@ updated.
Unparsable Bug: True (OOPS-...; http://bugs.example.com/show_bug.cgi?id=1)
Unknown: True (OOPS-...; http://bugs.example.com/show_bug.cgi?id=1)
- >>> capture.cleanUp()
-
Using `LookupTree` to map statuses
----------------------------------
diff --git a/lib/lp/services/webapp/doc/test_adapter_timeout.txt.disabled b/lib/lp/services/webapp/doc/test_adapter_timeout.txt.disabled
index aca50bb..5940282 100644
--- a/lib/lp/services/webapp/doc/test_adapter_timeout.txt.disabled
+++ b/lib/lp/services/webapp/doc/test_adapter_timeout.txt.disabled
@@ -82,9 +82,6 @@ exception, and a time machine.
Now we actually demonstrate the behaviour. The view did raise a TimeoutError.
- >>> from lp.testing.fixture import CaptureOops
- >>> capture = CaptureOops()
- >>> capture.setUp()
>>> browser = setupBrowser()
>>> browser.open('http://launchpad.test/doom_test')
Traceback (most recent call last):
@@ -98,12 +95,11 @@ The exception view did not.
The OOPS has the SQL from the main view.
- >>> print capture.oopses[0]['timeline']
+ >>> print oops_capture.oopses[-1]['timeline']
[(0, 0, 'SELECT ... FROM ... WHERE ...'), ...]
All that's left is to clean up after ourselves.
- >>> capture.cleanUp()
>>> sm.unregisterAdapter(
... DoomedView, required=(Interface, IBrowserRequest),
... provided=IBrowserView, name='doom_test')
diff --git a/lib/lp/soyuz/doc/gina.txt b/lib/lp/soyuz/doc/gina.txt
index 76b7098..1f773df 100644
--- a/lib/lp/soyuz/doc/gina.txt
+++ b/lib/lp/soyuz/doc/gina.txt
@@ -123,11 +123,9 @@ Let's set up the filesystem:
And give it a spin:
- >>> from lp.testing.fixture import CaptureOops
>>> gina_proc = [sys.executable, 'scripts/gina.py', '-q',
... 'hoary', 'breezy']
- >>> with CaptureOops():
- ... proc = subprocess.Popen(gina_proc, stderr=subprocess.PIPE)
+ >>> proc = subprocess.Popen(gina_proc, stderr=subprocess.PIPE)
Check STDERR for the errors we expected:
@@ -515,8 +513,7 @@ run.
>>> gina_proc = [sys.executable, 'scripts/gina.py', '-q',
... 'hoary', 'breezy']
- >>> with CaptureOops():
- ... proc = subprocess.Popen(gina_proc, stderr=subprocess.PIPE)
+ >>> proc = subprocess.Popen(gina_proc, stderr=subprocess.PIPE)
>>> print(proc.stderr.read())
ERROR Error processing package files for clearlooks
...
diff --git a/lib/lp/testing/systemdocs.py b/lib/lp/testing/systemdocs.py
index 5a34004..d609bc8 100644
--- a/lib/lp/testing/systemdocs.py
+++ b/lib/lp/testing/systemdocs.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Infrastructure for setting up doctests."""
@@ -43,6 +43,7 @@ from lp.testing import (
verifyObject,
)
from lp.testing.factory import LaunchpadObjectFactory
+from lp.testing.fixture import CaptureOops
from lp.testing.views import (
create_initialized_view,
create_view,
@@ -91,6 +92,41 @@ class StdoutHandler(Handler):
record.levelname, record.name, self.format(record)))
+def setUpStdoutLogging(test, prev_set_up=None,
+ stdout_logging_level=logging.INFO):
+ if prev_set_up is not None:
+ prev_set_up(test)
+ log = StdoutHandler('')
+ log.setLoggerLevel(stdout_logging_level)
+ log.install()
+ test.globs['log'] = log
+ # Store as instance attribute so we can uninstall it.
+ test._stdout_logger = log
+
+
+def tearDownStdoutLogging(test, prev_tear_down=None):
+ if prev_tear_down is not None:
+ prev_tear_down(test)
+ reset_logging()
+ test._stdout_logger.uninstall()
+
+
+def setUpOopsCapturing(test, prev_set_up=None):
+ oops_capture = CaptureOops()
+ oops_capture.setUp()
+ test.globs['oops_capture'] = oops_capture
+ # Store as instance attribute so we can clean it up.
+ test._oops_capture = oops_capture
+ if prev_set_up is not None:
+ prev_set_up(test)
+
+
+def tearDownOopsCapturing(test, prev_tear_down=None):
+ if prev_tear_down is not None:
+ prev_tear_down(test)
+ test._oops_capture.cleanUp()
+
+
def LayeredDocFileSuite(paths, id_extensions=None, **kw):
"""Create a DocFileSuite, optionally applying a layer to it.
@@ -119,29 +155,15 @@ def LayeredDocFileSuite(paths, id_extensions=None, **kw):
stdout_logging_level = kw.pop('stdout_logging_level', logging.INFO)
if stdout_logging:
- kw_setUp = kw.get('setUp')
-
- def setUp(test):
- if kw_setUp is not None:
- kw_setUp(test)
- log = StdoutHandler('')
- log.setLoggerLevel(stdout_logging_level)
- log.install()
- test.globs['log'] = log
- # Store as instance attribute so we can uninstall it.
- test._stdout_logger = log
-
- kw['setUp'] = setUp
-
- kw_tearDown = kw.get('tearDown')
-
- def tearDown(test):
- if kw_tearDown is not None:
- kw_tearDown(test)
- reset_logging()
- test._stdout_logger.uninstall()
-
- kw['tearDown'] = tearDown
+ kw['setUp'] = partial(
+ setUpStdoutLogging, prev_set_up=kw.get('setUp'),
+ stdout_logging_level=stdout_logging_level)
+ kw['tearDown'] = partial(
+ tearDownStdoutLogging, prev_tear_down=kw.get('tearDown'))
+
+ kw['setUp'] = partial(setUpOopsCapturing, prev_set_up=kw.get('setUp'))
+ kw['tearDown'] = partial(
+ tearDownOopsCapturing, prev_tear_down=kw.get('tearDown'))
layer = kw.pop('layer', None)
suite = doctest.DocFileSuite(*paths, **kw)
