tests: Replace httpretty with responses.

responses is an module analogous to httpretty for mocking external
URLs, with a very similar interface (potentially cleaner in that it
makes use of context managers).

The most important (in the moment) problem with httpretty is that it
breaks the ability to use redis in parts of code where httpretty is
enabled.  From more research, the module in general has tendency to
have various troublesome bugs with breaking URLs that it shouldn't be
affecting, caused by it working at the socket interface layer.  While
those issues could be fixed, responses seems to be less buggy (based
on both third-party reports like ckan/ckan#4755 and our own experience
in removing workarounds for bugs in httpretty) and is more actively
maintained.
This commit is contained in:
Mateusz Mandera 2020-01-22 17:41:49 +01:00 committed by Tim Abbott
parent 445a11753b
commit 8dd95bd057
6 changed files with 133 additions and 161 deletions

View File

@ -20,7 +20,7 @@ coverage
https://github.com/zulip/fakeldap/archive/ff32deaad34b91d5ba5735f314362c0456c92607.zip#egg=fakeldap==0.6.1zulip
# For testing mock http requests
httpretty
responses
# For sorting imports
isort

View File

@ -307,8 +307,6 @@ html2text==2019.9.26 \
httplib2==0.14.0 \
--hash=sha256:34537dcdd5e0f2386d29e0e2c6d4a1703a3b982d34c198a5102e6e5d6194b107 \
--hash=sha256:409fa5509298f739b34d5a652df762cb0042507dc93f6633e306b11289d6249d
httpretty==0.9.7 \
--hash=sha256:66216f26b9d2c52e81808f3e674a6fb65d4bf719721394a1a9be926177e55fbe
hypchat==0.21 \
--hash=sha256:ef37a9cd8103bb13ad772b28ba9223ca9d4278371e374450c3ea2918df70a8e9
hyper==0.7.0 \
@ -722,8 +720,7 @@ requests[security]==2.22.0 \
# via docker, hypchat, matrix-client, moto, premailer, pyoembed, python-digitalocean, python-gcm, python-twitter, requests-oauthlib, responses, social-auth-core, sphinx, stripe, transifex-client, twilio
responses==0.10.7 \
--hash=sha256:46d4e546a19fc6106bc7e804edd4551ef04690405e41e7e750ebc295d042623b \
--hash=sha256:93b1e0f2f960c0f3304ca4436856241d64c33683ef441431b9caf1d05d9d9e23 \
# via moto
--hash=sha256:93b1e0f2f960c0f3304ca4436856241d64c33683ef441431b9caf1d05d9d9e23
rsa==4.0 \
--hash=sha256:14ba45700ff1ec9eeb206a2ce76b32814958a98e372006c8fb76ba820211be66 \
--hash=sha256:1a836406405730121ae9823e19c6e806c62bbad73f890574fff50efa4122c487 \

View File

@ -4,18 +4,18 @@
from lib import sanity_check
sanity_check.check_venv(__file__)
from typing import List, Any
from typing import List, Iterator
import glob
import argparse
import contextlib
import mock
import os
import shlex
import sys
import subprocess
import tempfile
import ujson
import httplib2
import httpretty
import requests
import responses
import django
from django.conf import settings
@ -154,32 +154,27 @@ def write_failed_tests(failed_tests: List[str]) -> None:
with open(FAILED_TEST_PATH, 'w') as f:
ujson.dump(failed_tests, f)
def block_internet() -> None:
# We are blocking internet currently by assuming mostly any test would use
# httplib2 to access internet.
requests_orig = requests.request
@contextlib.contextmanager
def block_internet() -> Iterator[None]:
# Monkey-patching - responses library raises requests.ConnectionError when access to an unregistered URL
# is attempted. We want to replace that with our own exception, so that it propagates all the way:
with mock.patch.object(responses, 'ConnectionError', new=ZulipInternetBlockedError):
# We'll run all tests in this context manager. It'll cause an error to be raised (see above comment),
# if any code attempts to access the internet.
with responses.RequestsMock():
yield
def internet_guard_requests(*args: Any, **kwargs: Any) -> Any:
if httpretty.is_enabled():
return requests_orig(*args, **kwargs)
raise Exception("Outgoing network requests are not allowed in the Zulip tests. "
"More details and advice are available here:"
"https://zulip.readthedocs.io/en/latest/testing/testing.html#internet-access-inside-test-suites")
requests.request = internet_guard_requests
http2lib_request_orig = httplib2.Http.request
def internet_guard_httplib2(*args: Any, **kwargs: Any) -> Any:
if httpretty.is_enabled():
return http2lib_request_orig(*args, **kwargs)
raise Exception("Outgoing network requests are not allowed in the Zulip tests. "
"More details and advice are available here:"
"https://zulip.readthedocs.io/en/latest/testing/testing.html#internet-access-inside-test-suites")
httplib2.Http.request = internet_guard_httplib2
class ZulipInternetBlockedError(Exception):
def __init__(self, original_msg: str) -> None:
zulip_msg = (
"Outgoing network requests are not allowed in the Zulip tests. "
"More details and advice are available here:"
"https://zulip.readthedocs.io/en/latest/testing/testing.html#internet-access-inside-test-suites"
)
msg = "{}\nResponses library error message: {}".format(zulip_msg, original_msg)
super().__init__(msg)
def main() -> None:
block_internet()
TOOLS_DIR = os.path.dirname(os.path.abspath(__file__))
os.chdir(os.path.dirname(TOOLS_DIR))
sys.path.insert(0, os.path.dirname(TOOLS_DIR))
@ -381,11 +376,12 @@ def main() -> None:
else:
print("-- Running tests in serial mode.", flush=True)
test_runner = TestRunner(failfast=options.fatal_errors, verbosity=2,
parallel=parallel, reverse=options.reverse,
keepdb=True)
failures, failed_tests = test_runner.run_tests(suites, full_suite=full_suite,
include_webhooks=include_webhooks)
with block_internet():
test_runner = TestRunner(failfast=options.fatal_errors, verbosity=2,
parallel=parallel, reverse=options.reverse,
keepdb=True)
failures, failed_tests = test_runner.run_tests(suites, full_suite=full_suite,
include_webhooks=include_webhooks)
write_failed_tests(failed_tests)
templates_not_rendered = test_runner.get_shallow_tested_templates()

View File

@ -26,4 +26,4 @@ LATEST_RELEASE_ANNOUNCEMENT = "https://blog.zulip.org/2019/12/13/zulip-2-1-relea
# historical commits sharing the same major version, in which case a
# minor version bump suffices.
PROVISION_VERSION = '67.0'
PROVISION_VERSION = '67.1'

View File

@ -11,7 +11,7 @@ from typing import Any, Callable, Dict, List, Optional, Tuple
from django.core import signing
from django.urls import reverse
import httpretty
import responses
import ldap
import jwt
@ -384,38 +384,15 @@ class AuthBackendTest(ZulipTestCase):
backends_to_test = {
'google': {
'urls': [
{
'url': "https://accounts.google.com/o/oauth2/token",
'method': httpretty.POST,
'status': 200,
'body': json.dumps(token_data_dict),
},
{
'url': "https://www.googleapis.com/oauth2/v3/userinfo",
'method': httpretty.GET,
'status': 200,
'body': json.dumps(google_email_data),
},
# The limited process that we test here doesn't require mocking any urls.
],
'backend': GoogleAuthBackend,
},
'github': {
'urls': [
{
'url': "https://github.com/login/oauth/access_token",
'method': httpretty.POST,
'status': 200,
'body': json.dumps(token_data_dict),
},
{
'url': "https://api.github.com/user",
'method': httpretty.GET,
'status': 200,
'body': json.dumps(dict(email=user.email, name=user.full_name)),
},
{
'url': "https://api.github.com/user/emails",
'method': httpretty.GET,
'method': responses.GET,
'status': 200,
'body': json.dumps(github_email_data),
},
@ -448,42 +425,40 @@ class AuthBackendTest(ZulipTestCase):
return google_email_data['email']
for backend_name in backends_to_test:
httpretty.enable(allow_net_connect=False)
urls = backends_to_test[backend_name]['urls'] # type: List[Dict[str, Any]]
for details in urls:
httpretty.register_uri(
details['method'],
details['url'],
status=details['status'],
body=details['body'])
backend_class = backends_to_test[backend_name]['backend']
backend = backend_class()
backend.strategy = DjangoStrategy(storage=BaseDjangoStorage())
with responses.RequestsMock(assert_all_requests_are_fired=True) as requests_mock:
urls = backends_to_test[backend_name]['urls'] # type: List[Dict[str, Any]]
for details in urls:
requests_mock.add(
details['method'],
details['url'],
status=details['status'],
body=details['body'])
backend_class = backends_to_test[backend_name]['backend']
backend = backend_class()
backend.strategy = DjangoStrategy(storage=BaseDjangoStorage())
orig_authenticate = backend_class.authenticate
backend.authenticate = patched_authenticate
orig_get_verified_emails = backend_class.get_verified_emails
if backend_name == "google":
backend.get_verified_emails = patched_get_verified_emails
orig_authenticate = backend_class.authenticate
backend.authenticate = patched_authenticate
orig_get_verified_emails = backend_class.get_verified_emails
if backend_name == "google":
backend.get_verified_emails = patched_get_verified_emails
good_kwargs = dict(backend=backend, strategy=backend.strategy,
storage=backend.strategy.storage,
response=token_data_dict,
subdomain='zulip')
bad_kwargs = dict(subdomain='acme')
with mock.patch('zerver.views.auth.redirect_and_log_into_subdomain',
return_value=user):
self.verify_backend(backend,
good_kwargs=good_kwargs,
bad_kwargs=bad_kwargs)
bad_kwargs['subdomain'] = "zephyr"
self.verify_backend(backend,
good_kwargs=good_kwargs,
bad_kwargs=bad_kwargs)
backend.authenticate = orig_authenticate
backend.get_verified_emails = orig_get_verified_emails
httpretty.disable()
httpretty.reset()
good_kwargs = dict(backend=backend, strategy=backend.strategy,
storage=backend.strategy.storage,
response=token_data_dict,
subdomain='zulip')
bad_kwargs = dict(subdomain='acme')
with mock.patch('zerver.views.auth.redirect_and_log_into_subdomain',
return_value=user):
self.verify_backend(backend,
good_kwargs=good_kwargs,
bad_kwargs=bad_kwargs)
bad_kwargs['subdomain'] = "zephyr"
self.verify_backend(backend,
good_kwargs=good_kwargs,
bad_kwargs=bad_kwargs)
backend.authenticate = orig_authenticate
backend.get_verified_emails = orig_get_verified_emails
class CheckPasswordStrengthTest(ZulipTestCase):
def test_check_password_strength(self) -> None:
@ -530,7 +505,7 @@ class SocialAuthBase(ZulipTestCase):
from social_core.backends.utils import load_backends
load_backends(settings.AUTHENTICATION_BACKENDS, force_load=True)
def register_extra_endpoints(self,
def register_extra_endpoints(self, requests_mock: responses.RequestsMock,
account_data_dict: Dict[str, str],
**extra_data: Any) -> None:
pass
@ -602,66 +577,50 @@ class SocialAuthBase(ZulipTestCase):
# We register callbacks for the key URLs on Identity Provider that
# auth completion url will call
httpretty.enable(allow_net_connect=False)
httpretty.register_uri(
httpretty.POST,
self.ACCESS_TOKEN_URL,
match_querystring=False,
status=200,
body=json.dumps(token_data_dict))
httpretty.register_uri(
httpretty.GET,
self.USER_INFO_URL,
status=200,
body=json.dumps(account_data_dict)
)
self.register_extra_endpoints(account_data_dict, **extra_data)
parsed_url = urllib.parse.urlparse(result.url)
csrf_state = urllib.parse.parse_qs(parsed_url.query)['state']
result = self.client_get(self.AUTH_FINISH_URL,
dict(state=csrf_state), **headers)
if expect_choose_email_screen and result.status_code == 200:
# For authentication backends such as GitHub that
# successfully authenticate multiple email addresses,
# we'll have an additional screen where the user selects
# which email address to login using (this screen is a
# "partial" state of the python-social-auth pipeline).
#
# TODO: Generalize this testing code for use with other
# authentication backends; for now, we just assert that
# it's definitely the GitHub authentication backend.
self.assert_in_success_response(["Select account"], result)
assert self.AUTH_FINISH_URL == "/complete/github/"
# Testing hack: When the pipeline goes to the partial
# step, the below given URL is called again in the same
# test. If the below URL is not registered again as done
# below, the URL returns emails from previous tests. e.g
# email = 'invalid' may be one of the emails in the list
# in a test function followed by it. This is probably a
# bug in httpretty.
httpretty.disable()
httpretty.enable()
httpretty.register_uri(
httpretty.GET,
"https://api.github.com/user/emails",
with responses.RequestsMock(assert_all_requests_are_fired=False) as requests_mock:
requests_mock.add(
requests_mock.POST,
self.ACCESS_TOKEN_URL,
match_querystring=False,
status=200,
body=json.dumps(self.email_data)
body=json.dumps(token_data_dict))
requests_mock.add(
requests_mock.GET,
self.USER_INFO_URL,
status=200,
body=json.dumps(account_data_dict)
)
result = self.client_get(self.AUTH_FINISH_URL,
dict(state=csrf_state, email=account_data_dict['email']), **headers)
elif self.AUTH_FINISH_URL == "/complete/github/":
# We want to be explicit about when we expect a test to
# use the "choose email" screen, but of course we should
# only check for that screen with the GitHub backend,
# because this test code is shared with other
# authentication backends that structurally will never use
# that screen.
assert not expect_choose_email_screen
self.register_extra_endpoints(requests_mock, account_data_dict, **extra_data)
parsed_url = urllib.parse.urlparse(result.url)
csrf_state = urllib.parse.parse_qs(parsed_url.query)['state']
result = self.client_get(self.AUTH_FINISH_URL,
dict(state=csrf_state), **headers)
if expect_choose_email_screen and result.status_code == 200:
# For authentication backends such as GitHub that
# successfully authenticate multiple email addresses,
# we'll have an additional screen where the user selects
# which email address to login using (this screen is a
# "partial" state of the python-social-auth pipeline).
#
# TODO: Generalize this testing code for use with other
# authentication backends; for now, we just assert that
# it's definitely the GitHub authentication backend.
self.assert_in_success_response(["Select account"], result)
assert self.AUTH_FINISH_URL == "/complete/github/"
result = self.client_get(self.AUTH_FINISH_URL,
dict(state=csrf_state, email=account_data_dict['email']), **headers)
elif self.AUTH_FINISH_URL == "/complete/github/":
# We want to be explicit about when we expect a test to
# use the "choose email" screen, but of course we should
# only check for that screen with the GitHub backend,
# because this test code is shared with other
# authentication backends that structurally will never use
# that screen.
assert not expect_choose_email_screen
httpretty.disable()
return result
def test_social_auth_no_key(self) -> None:
@ -1340,7 +1299,7 @@ class GitHubAuthBackendTest(SocialAuthBase):
AUTH_FINISH_URL = "/complete/github/"
CONFIG_ERROR_URL = "/config-error/github"
def register_extra_endpoints(self,
def register_extra_endpoints(self, requests_mock: responses.RequestsMock,
account_data_dict: Dict[str, str],
**extra_data: Any) -> None:
# Keeping a verified email before the primary email makes sure
@ -1358,15 +1317,15 @@ class GitHubAuthBackendTest(SocialAuthBase):
]
email_data = extra_data.get("email_data", email_data)
httpretty.register_uri(
httpretty.GET,
requests_mock.add(
requests_mock.GET,
"https://api.github.com/user/emails",
status=200,
body=json.dumps(email_data)
)
httpretty.register_uri(
httpretty.GET,
requests_mock.add(
requests_mock.GET,
"https://api.github.com/teams/zulip-webapp/members/None",
status=200,
body=json.dumps(email_data)

View File

@ -0,0 +1,20 @@
from zerver.lib.test_classes import ZulipTestCase
import responses
import requests
class ResponsesTest(ZulipTestCase):
def test_responses(self) -> None:
# With our test setup, accessing the internet should be blocked.
with self.assertRaises(Exception):
result = requests.request('GET', 'https://www.google.com')
# A test can invoke its own responses.RequestsMock context manager
# and register URLs to mock, accessible from within the context.
with responses.RequestsMock() as requests_mock:
requests_mock.add(responses.GET, 'https://www.google.com',
body='{}', status=200,
content_type='application/json')
result = requests.request('GET', 'https://www.google.com')
self.assertEqual(result.status_code, 200)
self.assertEqual(result.text, '{}')