Merge lp:~james-w/python-oops-wsgi/start_response-exc_info into lp:python-oops-wsgi

[James Westby]

Hi,

As discussed exc_info shouldn't be passed as a keyword argument.

Thanks,

James

[lp:~james-w/python-oops-wsgi/start_response-exc_info]

36. By James Westby on 2012-01-25

Add the bug reference.

[Robert Collins (lifeless)]

35 + step = iterator.next()
36 + # the client pipe is closed or something - we discard the iterator
37 + del iterator
38 + gc.collect()
39 + return step

I think this is more complex than needed - a simple list() should do the trick; you probably want an assert on the content of the collected calls as well(e.g. nothing demonstrates that exc_info *was* supplied).

matchers.MatchesException(ValueError('Boom, yo')) will probably be an aid there.

Oh, and there is a typo in the test function name

review: Needs Fixing

[James Westby (james-w)]

Hi,

I think I have addressed your comments now.

Thanks,

James

[lp:~james-w/python-oops-wsgi/start_response-exc_info]

37. By James Westby on 2012-01-25

Fixes from review. Thanks Rob.

=== modified file 'oops_wsgi/middleware.py'
--- oops_wsgi/middleware.py	2011-12-09 04:06:45 +0000
+++ oops_wsgi/middleware.py	2012-01-25 03:04:24 +0000
@@ -193,7 +193,7 @@
                 headers = [('Content-Type', content_type)]
                 headers.append(('X-Oops-Id', str(report['id'])))
                 start_response(
-                    '500 Internal Server Error', headers, exc_info=exc_info)
+                    '500 Internal Server Error', headers, exc_info)
                 del exc_info
                 if error_render is not None:
                     return error_render(report)

=== modified file 'oops_wsgi/tests/test_middleware.py'
--- oops_wsgi/tests/test_middleware.py	2011-12-09 04:06:45 +0000
+++ oops_wsgi/tests/test_middleware.py	2012-01-25 03:04:24 +0000
@@ -31,6 +31,7 @@
 from testtools.matchers import (
     DocTestMatches,
     Equals,
+    MatchesException,
     MatchesListwise,
     Mismatch,
     MismatchesAll,
@@ -544,6 +545,22 @@
             Equals(expected_start_response),
             ]))
 
+    def test_calls_start_response_with_positional_exc_info(self):
+        def myapp(environ, start_response):
+            raise ValueError('boom, yo')
+        config = self.config_for_oopsing(capture_create=True)
+        app = make_app(myapp, config)
+        environ = self.make_outer_environ()
+        def start_response(*args, **kwargs):
+            if kwargs:
+                raise AssertionError(
+                    "start_response takes no kwargs: %s" % str(kwargs))
+            return self.calls.append
+        list(app(environ, start_response))
+        self.assertEqual(2, len(self.calls))
+        self.assertThat(self.calls[0]['exc_info'],
+            MatchesException(ValueError('boom, yo')))
+        self.assertThat(self.calls[1], MatchesOOPS({'value': 'boom, yo'}))
 
 
 class TestGeneratorTracker(TestCase):