launchpad-reviewers team mailing list archive

[James Westby <jw+debian@xxxxxxxxxxxxxxx>]

James Westby has proposed merging lp:~james-w/launchpad/devel into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Attach any oopses to the test results.

If you trigger an oops during a test it can be useful to access it, especially
when in a different thread/process to where the oops happens, such that the
traceback you get isn't the one that would be in the oops.
-- 
https://code.launchpad.net/~james-w/launchpad/devel/+merge/30776
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~james-w/launchpad/devel into lp:launchpad/devel.

=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py	2010-07-03 06:37:31 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py	2010-07-23 13:54:59 +0000
@@ -157,29 +157,34 @@
     def __repr__(self):
         return '<ErrorReport %s %s: %s>' % (self.id, self.type, self.value)
 
-    def write(self, fp):
-        fp.write('Oops-Id: %s\n' % _normalise_whitespace(self.id))
-        fp.write('Exception-Type: %s\n' % _normalise_whitespace(self.type))
-        fp.write('Exception-Value: %s\n' % _normalise_whitespace(self.value))
-        fp.write('Date: %s\n' % self.time.isoformat())
-        fp.write('Page-Id: %s\n' % _normalise_whitespace(self.pageid))
-        fp.write('Branch: %s\n' % self.branch_nick)
-        fp.write('Revision: %s\n' % self.revno)
-        fp.write('User: %s\n' % _normalise_whitespace(self.username))
-        fp.write('URL: %s\n' % _normalise_whitespace(self.url))
-        fp.write('Duration: %s\n' % self.duration)
-        fp.write('Informational: %s\n' % self.informational)
-        fp.write('\n')
+    def get_chunks(self):
+        chunks = []
+        chunks.append('Oops-Id: %s\n' % _normalise_whitespace(self.id))
+        chunks.append('Exception-Type: %s\n' % _normalise_whitespace(self.type))
+        chunks.append('Exception-Value: %s\n' % _normalise_whitespace(self.value))
+        chunks.append('Date: %s\n' % self.time.isoformat())
+        chunks.append('Page-Id: %s\n' % _normalise_whitespace(self.pageid))
+        chunks.append('Branch: %s\n' % self.branch_nick)
+        chunks.append('Revision: %s\n' % self.revno)
+        chunks.append('User: %s\n' % _normalise_whitespace(self.username))
+        chunks.append('URL: %s\n' % _normalise_whitespace(self.url))
+        chunks.append('Duration: %s\n' % self.duration)
+        chunks.append('Informational: %s\n' % self.informational)
+        chunks.append('\n')
         safe_chars = ';/\\?:@&+$, ()*!'
         for key, value in self.req_vars:
-            fp.write('%s=%s\n' % (urllib.quote(key, safe_chars),
+            chunks.append('%s=%s\n' % (urllib.quote(key, safe_chars),
                                   urllib.quote(value, safe_chars)))
-        fp.write('\n')
+        chunks.append('\n')
         for (start, end, database_id, statement) in self.db_statements:
-            fp.write('%05d-%05d@%s %s\n' % (
+            chunks.append('%05d-%05d@%s %s\n' % (
                 start, end, database_id, _normalise_whitespace(statement)))
-        fp.write('\n')
-        fp.write(self.tb_text)
+        chunks.append('\n')
+        chunks.append(self.tb_text)
+        return chunks
+
+    def write(self, fp):
+        fp.writelines(self.get_chunks())
 
     @classmethod
     def read(cls, fp):
@@ -215,8 +220,11 @@
             line = line.strip()
             if line == '':
                 break
-            start, end, db_id, statement = re.match(
-                r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line).groups()
+            match = re.match(
+                r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line)
+            assert match is not None, (
+                "Unable to interpret oops line: %s" % line)
+            start, end, db_id, statement = match.groups()
             if db_id is not None:
                 db_id = intern(db_id) # This string is repeated lots.
             statements.append(

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-07-22 13:41:50 +0000
+++ lib/lp/testing/__init__.py	2010-07-23 13:54:59 +0000
@@ -50,6 +50,7 @@
     ]
 
 from contextlib import contextmanager
+from cStringIO import StringIO
 from datetime import datetime, timedelta
 from inspect import getargspec, getmembers, getmro, isclass, ismethod
 import os
@@ -426,6 +427,17 @@
         config.push(name, "\n[%s]\n%s\n" % (section, body))
         self.addCleanup(config.pop, name)
 
+    def attachOopses(self):
+        if len(self.oopses) > 0:
+            content_type = testtools.content_type.ContentType(
+                "text", "plain", {"charset": "utf8"})
+            for (i, oops) in enumerate(self.oopses):
+                def get_oops_content():
+                    return oops.get_chunks()
+                content = testtools.content.Content(
+                    content_type, get_oops_content)
+                self.addDetail("oops-%d" % i, content)
+
     def setUp(self):
         testtools.TestCase.setUp(self)
         from lp.testing.factory import ObjectFactory
@@ -433,6 +445,7 @@
         # Record the oopses generated during the test run.
         self.oopses = []
         self.installFixture(ZopeEventHandlerFixture(self._recordOops))
+        self.addCleanup(self.attachOopses)
 
     @adapter(ErrorReportEvent)
     def _recordOops(self, event):

=== modified file 'lib/lp/testing/tests/test_testcase.py'
--- lib/lp/testing/tests/test_testcase.py	2009-09-15 05:58:02 +0000
+++ lib/lp/testing/tests/test_testcase.py	2010-07-23 13:54:59 +0000
@@ -5,12 +5,17 @@
 
 __metaclass__ = type
 
+from StringIO import StringIO
+import sys
+
 from storm.store import Store
 import unittest
 
 from zope.component import getUtility
 
-from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.launchpad.webapp import errorlog
+from canonical.testing.layers import (
+    DatabaseFunctionalLayer, FunctionalLayer)
 
 from lp.code.interfaces.branch import IBranchSet
 from lp.testing import record_statements, TestCaseWithFactory
@@ -42,6 +47,55 @@
         self.assertStatementCount(1, getattr, branch, "owner")
 
 
+class TestCaptureOops(TestCaseWithFactory):
+
+    layer = FunctionalLayer
+
+    def trigger_oops(self):
+        try:
+            raise AssertionError("Exception to get a traceback.")
+        except AssertionError:
+            errorlog.globalErrorUtility.raising(sys.exc_info())
+
+    def test_no_oops_gives_no_details(self):
+        self.assertEqual(0, len(self.oopses))
+        self.attachOopses()
+        self.assertEqual(
+            0, len([a for a in self.getDetails() if "oops" in a]))
+
+    def test_one_oops_gives_one_detail(self):
+        self.assertEqual(0, len(self.oopses))
+        self.trigger_oops()
+        self.attachOopses()
+        self.assertEqual(
+            ["oops-0"], [a for a in self.getDetails() if "oops" in a])
+
+    def test_two_oops_gives_two_details(self):
+        self.assertEqual(0, len(self.oopses))
+        self.trigger_oops()
+        self.trigger_oops()
+        self.attachOopses()
+        self.assertEqual(
+            ["oops-0", "oops-1"],
+            sorted([a for a in self.getDetails() if "oops" in a]))
+
+    def test_oops_content(self):
+        self.assertEqual(0, len(self.oopses))
+        self.trigger_oops()
+        self.attachOopses()
+        oops = errorlog.globalErrorUtility.getLastOopsReport()
+        # We have to serialise and read in again, as that is what
+        # getLastOopsReport does, and doing so changes whether the
+        # timezone is in the timestamp
+        content = StringIO()
+        content.writelines(self.getDetails()['oops-0'].iter_text())
+        content.seek(0)
+        from_details = errorlog.ErrorReport.read(content)
+        self.assertEqual(
+            oops.get_chunks(),
+            from_details.get_chunks())
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)