launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24096
[Merge] ~cjwatson/launchpad:remove-most-wsgi-intercept into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:remove-most-wsgi-intercept into launchpad:master with ~cjwatson/launchpad:simplify-appserverlayer-browser as a prerequisite.
Commit message:
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.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/374881
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-most-wsgi-intercept into launchpad:master.
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 4e680cf..6ff5141 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.
