Merge ~cjwatson/launchpad:remove-most-wsgi-intercept into launchpad:master

[Colin Watson]

Avoid most uses of wsgi_intercept

zope.testbrowser 4.0.0 uses WebTest instead of wsgi_intercept, so we
also want to stop using wsgi_intercept for our own purposes. We can
point the WSGI test browser directly at an appropriate WSGI application
instead.

[Ioana Lasc (ilasc)]

review: Approve

diff --git a/lib/launchpad_loggerhead/tests.py b/lib/launchpad_loggerhead/tests.py
index f332518..31cb5a7 100644
--- a/lib/launchpad_loggerhead/tests.py
+++ b/lib/launchpad_loggerhead/tests.py
@@ -1,7 +1,6 @@
 # Copyright 2010-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-import lazr.uri
 from paste.httpexceptions import HTTPExceptionHandler
 import requests
 from six.moves.urllib_parse import (
@@ -11,12 +10,7 @@ from six.moves.urllib_parse import (
 import soupmatchers
 from testtools.content import Content
 from testtools.content_type import UTF8_TEXT
-import wsgi_intercept
-from wsgi_intercept.urllib2_intercept import (
-    install_opener,
-    uninstall_opener,
-    )
-import wsgi_intercept.zope_testbrowser
+from zope.testbrowser.wsgi import Browser
 from zope.security.proxy import removeSecurityProxy
 
 from launchpad_loggerhead.app import RootApp
@@ -50,12 +44,6 @@ def session_scribbler(app, test):
     return scribble
 
 
-def dummy_destination(environ, start_response):
-    """Return a fake response."""
-    start_response('200 OK', [('Content-type', 'text/plain')])
-    return ['This is a dummy destination.\n']
-
-
 class SimpleLogInRootApp(RootApp):
     """A mock root app that doesn't require open id."""
     def _complete_login(self, environ, start_response):
@@ -63,53 +51,35 @@ class SimpleLogInRootApp(RootApp):
         start_response('200 OK', [('Content-type', 'text/plain')])
         return ['\n']
 
+    def __call__(self, environ, start_response):
+        codebrowse_netloc = urlsplit(
+            config.codehosting.secure_codebrowse_root).netloc
+        if environ['HTTP_HOST'] == codebrowse_netloc:
+            return RootApp.__call__(self, environ, start_response)
+        else:
+            # Return a fake response.
+            start_response('200 OK', [('Content-type', 'text/plain')])
+            return ['This is a dummy destination.\n']
+
 
 class TestLogout(TestCase):
     layer = DatabaseFunctionalLayer
 
-    def intercept(self, uri, app):
-        """Install wsgi interceptors for the uri, app tuple."""
-        if isinstance(uri, basestring):
-            uri = lazr.uri.URI(uri)
-        port = uri.port
-        if port is None:
-            if uri.scheme == 'http':
-                port = 80
-            elif uri.scheme == 'https':
-                port = 443
-            else:
-                raise NotImplementedError(uri.scheme)
-        else:
-            port = int(port)
-        wsgi_intercept.add_wsgi_intercept(uri.host, port, lambda: app)
-        self.intercepted.append((uri.host, port))
-
     def setUp(self):
         TestCase.setUp(self)
-        self.intercepted = []
         self.session = None
-        self.root = app = SimpleLogInRootApp(SESSION_VAR)
+        app = SimpleLogInRootApp(SESSION_VAR)
         app = session_scribbler(app, self)
         app = HTTPExceptionHandler(app)
         app = SessionHandler(app, SESSION_VAR, SECRET)
         self.cookie_name = app.cookie_handler.cookie_name
-        self.intercept(config.codehosting.secure_codebrowse_root, app)
-        self.intercept(allvhosts.configs['mainsite'].rooturl,
-                       dummy_destination)
-        install_opener()
-        self.browser = wsgi_intercept.zope_testbrowser.WSGI_Browser()
+        self.browser = Browser(wsgi_app=app)
         # We want to pretend we are not a robot, or else mechanize will honor
         # robots.txt.
         self.browser.mech_browser.set_handle_robots(False)
         self.browser.open(
             config.codehosting.secure_codebrowse_root + '+login')
 
-    def tearDown(self):
-        uninstall_opener()
-        for host, port in self.intercepted:
-            wsgi_intercept.remove_wsgi_intercept(host, port)
-        TestCase.tearDown(self)
-
     def testLoggerheadLogout(self):
         # We start logged in as 'bob'.
         self.assertEqual(self.session['user'], 'bob')
@@ -143,8 +113,7 @@ class TestLogout(TestCase):
         # TestLoginAndLogout.test_CookieLogoutPage).
 
         # Here, we will have a more useless example of the basic machinery.
-        dummy_root = 'http://dummy.dev/'
-        self.intercept(dummy_root, dummy_destination)
+        dummy_root = 'http://dummy.test/'
         self.browser.open(
             config.codehosting.secure_codebrowse_root +
             '+logout?' + urlencode(dict(next_to=dummy_root + '+logout')))
diff --git a/lib/lp/registry/stories/webservice/xx-distribution-mirror.txt b/lib/lp/registry/stories/webservice/xx-distribution-mirror.txt
index edcef0e..9e4c932 100644
--- a/lib/lp/registry/stories/webservice/xx-distribution-mirror.txt
+++ b/lib/lp/registry/stories/webservice/xx-distribution-mirror.txt
@@ -165,15 +165,15 @@ Now trying to set the owner using Sample Person's webservice is not authorized.
     ... }
     >>> response = test_webservice.patch(
     ...     canonical_archive['self_link'], 'application/json', dumps(patch))
-    >>> print response.getheader('status')
-    401 Unauthorized
+    >>> response.status
+    401
 
 But if we use Karl, the mirror listing admin's, webservice, we can update the owner.
 
     >>> response = karl_webservice.patch(
     ...     canonical_archive['self_link'], 'application/json', dumps(patch))
-    >>> print response.getheader('status')
-    209 Content Returned
+    >>> response.status
+    209
 
     >>> patched_canonical_archive = response.jsonBody()
     >>> print patched_canonical_archive['owner_link']
@@ -193,7 +193,6 @@ Some attributes are read-only via the API:
     ...     canonical_releases['self_link'], 'application/json', dumps(patch))
     >>> print response
     HTTP/1.1 400 Bad Request
-    Status: 400 Bad Request
     ...
     enabled: You tried to modify a read-only attribute.
     date_reviewed: You tried to modify a read-only attribute.
@@ -211,8 +210,8 @@ While others can be set with the appropriate authorization:
     ... }
     >>> response = test_webservice.patch(
     ...     canonical_releases['self_link'], 'application/json', dumps(patch))
-    >>> print response.getheader('status')
-    401 Unauthorized
+    >>> response.status
+    401
 
     >>> response = karl_webservice.patch(
     ...     canonical_releases['self_link'], 'application/json', dumps(patch)).jsonBody()
diff --git a/lib/lp/services/webapp/tests/test_error.py b/lib/lp/services/webapp/tests/test_error.py
index 7ad4e83..3170ae3 100644
--- a/lib/lp/services/webapp/tests/test_error.py
+++ b/lib/lp/services/webapp/tests/test_error.py
@@ -12,6 +12,7 @@ import urllib2
 
 from fixtures import FakeLogger
 import psycopg2
+from six.moves.urllib.error import HTTPError
 from storm.exceptions import (
     DisconnectionError,
     OperationalError,
@@ -26,6 +27,7 @@ from testtools.matchers import (
 import transaction
 from zope.interface import Interface
 from zope.publisher.interfaces.browser import IDefaultBrowserLayer
+from zope.testbrowser.wsgi import Browser
 
 from lp.services.webapp.error import (
     DisconnectionErrorView,
@@ -37,12 +39,12 @@ from lp.testing import TestCase
 from lp.testing.fixture import (
     CaptureOops,
     PGBouncerFixture,
-    Urllib2Fixture,
     ZopeAdapterFixture,
     )
 from lp.testing.layers import (
     DatabaseLayer,
     LaunchpadFunctionalLayer,
+    wsgi_application,
     )
 from lp.testing.matchers import Contains
 
@@ -78,8 +80,8 @@ class TestDatabaseErrorViews(TestCase):
 
     def getHTTPError(self, url):
         try:
-            urllib2.urlopen(url)
-        except urllib2.HTTPError as error:
+            Browser(wsgi_app=wsgi_application).open(url)
+        except HTTPError as error:
             return error
         else:
             self.fail("We should have gotten an HTTP error")
@@ -103,13 +105,14 @@ class TestDatabaseErrorViews(TestCase):
     def retryConnection(self, url, bouncer, retries=60):
         """Retry to connect to *url* for *retries* times.
 
-        Return the file-like object returned by *urllib2.urlopen(url)*.
-        Raise a TimeoutException if the connection can not be established.
+        Raise a TimeoutException if the connection cannot be established.
         """
+        browser = Browser(wsgi_app=wsgi_application)
         for i in range(retries):
             try:
-                return urllib2.urlopen(url)
-            except urllib2.HTTPError as e:
+                browser.open(url)
+                return
+            except HTTPError as e:
                 if e.code != httplib.SERVICE_UNAVAILABLE:
                     raise
             time.sleep(1)
@@ -122,7 +125,6 @@ class TestDatabaseErrorViews(TestCase):
     def test_disconnectionerror_view_integration(self):
         # Test setup.
         self.useFixture(FakeLogger('SiteError', level=logging.CRITICAL))
-        self.useFixture(Urllib2Fixture())
         bouncer = PGBouncerFixture()
         # XXX gary bug=974617, bug=1011847, bug=504291 2011-07-03:
         # In parallel tests, we are rarely encountering instances of
@@ -230,7 +232,6 @@ class TestDatabaseErrorViews(TestCase):
     def test_operationalerror_view_integration(self):
         # Test setup.
         self.useFixture(FakeLogger('SiteError', level=logging.CRITICAL))
-        self.useFixture(Urllib2Fixture())
 
         class BrokenView(object):
             """A view that raises an OperationalError"""
diff --git a/lib/lp/services/webservice/stories/xx-service.txt b/lib/lp/services/webservice/stories/xx-service.txt
index cfe2c32..ee15cd4 100644
--- a/lib/lp/services/webservice/stories/xx-service.txt
+++ b/lib/lp/services/webservice/stories/xx-service.txt
@@ -54,8 +54,8 @@ consumer keys.
 
     >>> caller = LaunchpadWebServiceCaller(u'new-consumer', u'access-key')
     >>> response = caller.get(root)
-    >>> print response.getheader('status')
-    401 Unauthorized
+    >>> response.status
+    401
     >>> print response.body
     Unknown consumer (new-consumer).
 
@@ -73,14 +73,14 @@ doesn't recognize the client.
 
     >>> caller = LaunchpadWebServiceCaller(u'another-new-consumer', u'')
     >>> response = caller.get(root)
-    >>> print response.getheader('status')
-    200 Ok
+    >>> response.status
+    200
 
 Anonymous requests can't access certain data.
 
     >>> response = anon_webservice.get(body['me_link'])
-    >>> print response.getheader('status')
-    401 Unauthorized
+    >>> response.status
+    401
     >>> print response.body
     You need to be logged in to view this URL.
 
@@ -90,8 +90,8 @@ Anonymous requests can't change the dataset.
     >>> data = simplejson.dumps({'display_name' : "This won't work"})
     >>> response = anon_webservice.patch(root + "/~salgado",
     ...     'application/json', data)
-    >>> print response.getheader('status')
-    401 Unauthorized
+    >>> response.status
+    401
     >>> print response.body
     (<Person at...>, 'display_name', 'launchpad.Edit')
 
diff --git a/lib/lp/soyuz/stories/webservice/xx-archive.txt b/lib/lp/soyuz/stories/webservice/xx-archive.txt
index 57bb1f5..7acd47c 100644
--- a/lib/lp/soyuz/stories/webservice/xx-archive.txt
+++ b/lib/lp/soyuz/stories/webservice/xx-archive.txt
@@ -874,12 +874,12 @@ methods.
 
     >>> response = webservice.named_get(
     ...     cprov_archive['self_link'], 'getPublishedSources')
-    >>> print(response.getheader('status'))
-    200 Ok
+    >>> response.status
+    200
     >>> response = webservice.named_get(
     ...     cprov_archive['self_link'], 'getPublishedBinaries')
-    >>> print(response.getheader('status'))
-    200 Ok
+    >>> response.status
+    200
 
 If either method is called with the version parameter, the name must
 be specified too, otherwise it is considered a bad webservice
@@ -887,13 +887,13 @@ request.
 
     >>> response = webservice.named_get(
     ...     cprov_archive['self_link'], 'getPublishedSources', version='1.0')
-    >>> print(response.getheader('status'))
-    400 Bad Request
+    >>> response.status
+    400
     >>> response = webservice.named_get(
     ...     cprov_archive['self_link'], 'getPublishedBinaries',
     ...     version='1.0')
-    >>> print(response.getheader('status'))
-    400 Bad Request
+    >>> response.status
+    400
 
 We don't have to specify any filters when getting published sources:
 
diff --git a/lib/lp/soyuz/stories/webservice/xx-person-createppa.txt b/lib/lp/soyuz/stories/webservice/xx-person-createppa.txt
index 3f4f9a1..085d8c8 100644
--- a/lib/lp/soyuz/stories/webservice/xx-person-createppa.txt
+++ b/lib/lp/soyuz/stories/webservice/xx-person-createppa.txt
@@ -22,7 +22,6 @@
   ...     name='yay', displayname='My shiny new PPA',
   ...     description='Shinyness!'))
   HTTP/1.1 201 Created
-  Status: 201
   ...
   Location: http://api.launchpad.test/.../+archive/ubuntu/yay
   ...
@@ -32,7 +31,6 @@
   ...     displayname='My shiny new PPA', description='Shinyness!',
   ...     ))
   HTTP/1.1 400 Bad Request
-  Status: 400 Bad Request
   ...
   A PPA cannot have the same name as its distribution.
 
@@ -52,7 +50,6 @@ They aren't a commercial admin, so they cannot create private PPAs.
   ...     private=True,
   ...     ))
   HTTP/1.1 400 Bad Request
-  Status: 400
   ...
   ... is not allowed to make private PPAs
 
@@ -78,7 +75,6 @@ Once they have commercial access, they can create private PPAs:
   ...     private=True,
   ...     ))
   HTTP/1.1 201 Created
-  Status: 201
   ...
   Location: http://api.launchpad.test/.../+archive/ubuntu/secret
   ...
@@ -103,7 +99,6 @@ It's possible to create PPAs for all sorts of distributions.
   ...     ppa_owner['self_link'], 'createPPA', {}, distribution='/ubuntutest',
   ...     name='ppa'))
   HTTP/1.1 201 Created
-  Status: 201
   ...
   Location: http://api.launchpad.test/.../+archive/ubuntutest/ppa
   ...
@@ -114,7 +109,6 @@ But not for distributions that don't have PPAs enabled.
   ...     ppa_owner['self_link'], 'createPPA', {}, distribution='/redhat',
   ...     name='ppa'))
   HTTP/1.1 400 Bad Request
-  Status: 400
   ...
   Red Hat does not support PPAs.
 
@@ -128,7 +122,6 @@ respectively.
   >>> print(ppa_owner_webservice.named_post(
   ...     ppa_owner['self_link'], 'createPPA', {}))
   HTTP/1.1 201 Created
-  Status: 201
   ...
   Location: http://api.launchpad.test/.../+archive/ubuntu/ppa
   ...
diff --git a/lib/lp/testing/fixture.py b/lib/lp/testing/fixture.py
index 557a385..45d3b50 100644
--- a/lib/lp/testing/fixture.py
+++ b/lib/lp/testing/fixture.py
@@ -10,7 +10,6 @@ __all__ = [
     'DisableTriggerFixture',
     'PGBouncerFixture',
     'PGNotReadyError',
-    'Urllib2Fixture',
     'ZopeAdapterFixture',
     'ZopeEventHandlerFixture',
     'ZopeUtilityFixture',
@@ -31,14 +30,6 @@ from lazr.restful.utils import get_current_browser_request
 import oops
 import oops_amqp
 import pgbouncer.fixture
-from wsgi_intercept import (
-    add_wsgi_intercept,
-    remove_wsgi_intercept,
-    )
-from wsgi_intercept.urllib2_intercept import (
-    install_opener,
-    uninstall_opener,
-    )
 from zope.component import (
     adapter,
     getGlobalSiteManager,
@@ -254,22 +245,6 @@ class ZopeUtilityFixture(Fixture):
                 gsm.registerUtility, original, self.intf, self.name)
 
 
-class Urllib2Fixture(Fixture):
-    """Let tests use urllib to connect to an in-process Launchpad.
-
-    Initially this only supports connecting to launchpad.test because
-    that is all that is needed.  Later work could connect all
-    sub-hosts (e.g. bugs.launchpad.test)."""
-
-    def _setUp(self):
-        # Work around circular import.
-        from lp.testing.layers import wsgi_application
-        add_wsgi_intercept('launchpad.test', 80, lambda: wsgi_application)
-        self.addCleanup(remove_wsgi_intercept, 'launchpad.test', 80)
-        install_opener()
-        self.addCleanup(uninstall_opener)
-
-
 class CaptureOops(Fixture):
     """Capture OOPSes notified via zope event notification.
 
diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
index 22a510f..984e87b 100644
--- a/lib/lp/testing/layers.py
+++ b/lib/lp/testing/layers.py
@@ -1020,12 +1020,9 @@ def wsgi_application(environ, start_response):
     request = zope.publisher.publish.publish(
         request, handle_errors=handle_errors)
     response = request.response
-    # We sort these, and then put the status first, because
-    # zope.app.testing.testbrowser does--and because it makes it easier to
-    # write reliable tests.
+    # We sort these because it makes it easier to write reliable tests.
     headers = sorted(response.getHeaders())
     status = response.getStatusString()
-    headers.insert(0, ('Status', status))
     # Start the WSGI server response.
     start_response(status, headers)
     # Return the result body iterable.