From 27edc183306f8ac8d1ca61075980866e42ff3ab7 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 24 Feb 2020 17:53:12 -0800 Subject: [PATCH] test_classes: Use realistic web and mobile User-Agent strings. This fixes a confusing aspect of how our automated tests worked previously, where we'd almost all HTTP requests in the unlikely configuration with no User-Agent string specified. We need to adjust query counts in a few tests that now are a bit cheaper because they now can take advantage of a Client object created in server_initialization.py in `process_client`. --- zerver/lib/test_classes.py | 16 ++++++++++++++++ zerver/tests/test_compatibility.py | 2 +- zerver/tests/test_subs.py | 8 ++++---- zerver/tests/test_tornado.py | 6 +++++- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index da6e876481..751db3d84a 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -135,6 +135,22 @@ class ZulipTestCase(TestCase): elif 'HTTP_HOST' not in kwargs: kwargs['HTTP_HOST'] = Realm.host_for_subdomain(self.DEFAULT_SUBDOMAIN) + # set User-Agent + if 'HTTP_AUTHORIZATION' in kwargs: + # An API request; use mobile as the default user agent + default_user_agent = "ZulipMobile/26.22.145 (iOS 10.3.1)" + else: + # A webapp request; use a browser User-Agent string. + default_user_agent = ("Mozilla/5.0 (Windows NT 10.0; Win64; x64) " + + "AppleWebKit/537.36 (KHTML, like Gecko) " + + "Chrome/79.0.3945.130 Safari/537.36") + if kwargs.get('skip_user_agent'): + # Provide a way to disable setting User-Agent if desired. + assert 'HTTP_USER_AGENT' not in kwargs + del kwargs['skip_user_agent'] + elif 'HTTP_USER_AGENT' not in kwargs: + kwargs['HTTP_USER_AGENT'] = default_user_agent + @instrument_url def client_patch(self, url: str, info: Dict[str, Any]={}, **kwargs: Any) -> HttpResponse: """ diff --git a/zerver/tests/test_compatibility.py b/zerver/tests/test_compatibility.py index 0fe675d093..7b9817ccee 100644 --- a/zerver/tests/test_compatibility.py +++ b/zerver/tests/test_compatibility.py @@ -78,7 +78,7 @@ class CompatibilityTest(ZulipTestCase): '''.strip().split('\n') if case] def test_compatibility_without_user_agent(self) -> None: - result = self.client_get("/compatibility") + result = self.client_get("/compatibility", skip_user_agent=True) self.assert_json_error(result, 'User-Agent header missing from request') def test_compatibility(self) -> None: diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index d0fd955d22..78edb95e2c 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -2443,7 +2443,7 @@ class SubscriptionAPITest(ZulipTestCase): streams_to_sub, dict(principals=ujson.dumps([user1.email, user2.email])), ) - self.assert_length(queries, 43) + self.assert_length(queries, 42) self.assert_length(events, 7) for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]: @@ -2817,7 +2817,7 @@ class SubscriptionAPITest(ZulipTestCase): # Make sure Zephyr mirroring realms such as MIT do not get # any tornado subscription events self.assert_length(events, 0) - self.assert_length(queries, 8) + self.assert_length(queries, 7) events = [] with tornado_redirected_to_list(events): @@ -2843,7 +2843,7 @@ class SubscriptionAPITest(ZulipTestCase): dict(principals=ujson.dumps([self.test_email])), ) # Make sure we don't make O(streams) queries - self.assert_length(queries, 19) + self.assert_length(queries, 18) def test_subscriptions_add_for_principal(self) -> None: """ @@ -3232,7 +3232,7 @@ class SubscriptionAPITest(ZulipTestCase): [new_streams[0]], dict(principals=ujson.dumps([user1.email, user2.email])), ) - self.assert_length(queries, 43) + self.assert_length(queries, 42) # Test creating private stream. with queries_captured() as queries: diff --git a/zerver/tests/test_tornado.py b/zerver/tests/test_tornado.py index 50e95ba01d..607229561a 100644 --- a/zerver/tests/test_tornado.py +++ b/zerver/tests/test_tornado.py @@ -37,6 +37,7 @@ class TornadoWebTestCase(AsyncHTTPTestCase, ZulipTestCase): def client_get(self, path: str, **kwargs: Any) -> HTTPResponse: self.add_session_cookie(kwargs) + kwargs['skip_user_agent'] = True self.set_http_headers(kwargs) if 'HTTP_HOST' in kwargs: kwargs['headers']['Host'] = kwargs['HTTP_HOST'] @@ -45,6 +46,7 @@ class TornadoWebTestCase(AsyncHTTPTestCase, ZulipTestCase): def fetch_async(self, method: str, path: str, **kwargs: Any) -> None: self.add_session_cookie(kwargs) + kwargs['skip_user_agent'] = True self.set_http_headers(kwargs) if 'HTTP_HOST' in kwargs: kwargs['headers']['Host'] = kwargs['HTTP_HOST'] @@ -57,6 +59,7 @@ class TornadoWebTestCase(AsyncHTTPTestCase, ZulipTestCase): ) def client_get_async(self, path: str, **kwargs: Any) -> None: + kwargs['skip_user_agent'] = True self.set_http_headers(kwargs) self.fetch_async('GET', path, **kwargs) @@ -78,7 +81,8 @@ class TornadoWebTestCase(AsyncHTTPTestCase, ZulipTestCase): kwargs['headers'] = headers def create_queue(self, **kwargs: Any) -> str: - response = self.client_get('/json/events?dont_block=true', subdomain="zulip") + response = self.client_get('/json/events?dont_block=true', subdomain="zulip", + skip_user_agent=True) self.assertEqual(response.code, 200) body = ujson.loads(response.body) self.assertEqual(body['events'], [])