From 48800e657f8a28a6ca75db45d6c44fc755f8f109 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= <98187+Tronic@users.noreply.github.com> Date: Sat, 28 Mar 2020 20:43:14 +0200 Subject: [PATCH] Deprecation and test cleanup (#1818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove remove_route, deprecated in 19.6. * No need for py35 compat anymore. * Rewrite asyncio.coroutines with async/await. * Remove deprecated request.raw_args. * response.text() takes str only: avoid deprecation warning in all but one test. * Remove unused import. * Revert unnecessary deprecation warning. * Remove apparently unnecessary py38 compat. * Avoid asyncio.Task.all_tasks deprecation warning. * Avoid warning on a test that tests deprecated response.text(int). * Add pytest-asyncio to tox deps. * Run the coroutine returned by AsyncioServer.close. Co-authored-by: L. Kärkkäinen --- sanic/app.py | 30 +---------- sanic/request.py | 13 ----- sanic/response.py | 5 -- sanic/router.py | 31 ------------ sanic/server.py | 5 +- tests/test_app.py | 3 +- tests/test_asgi.py | 15 +++++- tests/test_cookies.py | 2 +- tests/test_create_task.py | 4 +- tests/test_requests.py | 27 ---------- tests/test_response.py | 1 + tests/test_routes.py | 102 -------------------------------------- tests/test_static.py | 14 ------ tests/test_vhosts.py | 14 ------ tests/test_worker.py | 14 +++--- tox.ini | 1 + 16 files changed, 28 insertions(+), 253 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 60f845b6..c38f4f7c 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -507,12 +507,7 @@ class Sanic: if self.asgi: ws = request.transport.get_websocket_connection() else: - try: - protocol = request.transport.get_protocol() - except AttributeError: - # On Python3.5 the Transport classes in asyncio do not - # have a get_protocol() method as in uvloop - protocol = request.transport._protocol + protocol = request.transport.get_protocol() protocol.app = self ws = await protocol.websocket_handshake( @@ -599,29 +594,6 @@ class Sanic: self.websocket_enabled = enable - def remove_route(self, uri, clean_cache=True, host=None): - """ - This method provides the app user a mechanism by which an already - existing route can be removed from the :class:`Sanic` object - - .. warning:: - remove_route is deprecated in v19.06 and will be removed - from future versions. - - :param uri: URL Path to be removed from the app - :param clean_cache: Instruct sanic if it needs to clean up the LRU - route cache - :param host: IP address or FQDN specific to the host - :return: None - """ - warnings.warn( - "remove_route is deprecated and will be removed " - "from future versions.", - DeprecationWarning, - stacklevel=2, - ) - self.router.remove(uri, clean_cache, host) - # Decorator def exception(self, *exceptions): """Decorate a function to be registered as a handler for exceptions diff --git a/sanic/request.py b/sanic/request.py index dca15901..53706eab 100644 --- a/sanic/request.py +++ b/sanic/request.py @@ -1,6 +1,5 @@ import asyncio import email.utils -import warnings from collections import defaultdict, namedtuple from http.cookies import SimpleCookie @@ -284,18 +283,6 @@ class Request: args = property(get_args) - @property - def raw_args(self) -> dict: - if self.app.debug: # pragma: no cover - warnings.simplefilter("default") - warnings.warn( - "Use of raw_args will be deprecated in " - "the future versions. Please use args or query_args " - "properties instead", - DeprecationWarning, - ) - return {k: v[0] for k, v in self.args.items()} - def get_query_args( self, keep_blank_values: bool = False, diff --git a/sanic/response.py b/sanic/response.py index 7b7c8521..2d268464 100644 --- a/sanic/response.py +++ b/sanic/response.py @@ -43,11 +43,6 @@ class BaseHTTPResponse: ): """.. deprecated:: 20.3: This function is not public API and will be removed.""" - if version != "1.1": - warnings.warn( - "Only HTTP/1.1 is currently supported (got {version})", - DeprecationWarning, - ) # self.headers get priority over content_type if self.content_type and "Content-Type" not in self.headers: diff --git a/sanic/router.py b/sanic/router.py index 2d8817a3..013f88d7 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -352,37 +352,6 @@ class Router: else: return -1, None - def remove(self, uri, clean_cache=True, host=None): - if host is not None: - uri = host + uri - try: - route = self.routes_all.pop(uri) - for handler_name, pairs in self.routes_names.items(): - if pairs[0] == uri: - self.routes_names.pop(handler_name) - break - - for handler_name, pairs in self.routes_static_files.items(): - if pairs[0] == uri: - self.routes_static_files.pop(handler_name) - break - - except KeyError: - raise RouteDoesNotExist("Route was not registered: {}".format(uri)) - - if route in self.routes_always_check: - self.routes_always_check.remove(route) - elif ( - url_hash(uri) in self.routes_dynamic - and route in self.routes_dynamic[url_hash(uri)] - ): - self.routes_dynamic[url_hash(uri)].remove(route) - else: - self.routes_static.pop(uri) - - if clean_cache: - self._get.cache_clear() - @lru_cache(maxsize=ROUTER_CACHE_SIZE) def find_route_by_view_name(self, view_name, name=None): """Find a route in the router based on the specified view name. diff --git a/sanic/server.py b/sanic/server.py index 31dedd40..003ddc4d 100644 --- a/sanic/server.py +++ b/sanic/server.py @@ -973,10 +973,7 @@ def serve( else: conn.close() - if sys.version_info.minor >= 8: - _shutdown = asyncio.gather(*coros, loop=loop) - else: - _shutdown = asyncio.gather(*coros) + _shutdown = asyncio.gather(*coros) loop.run_until_complete(_shutdown) trigger_events(after_stop, loop) diff --git a/tests/test_app.py b/tests/test_app.py index c9cb8329..20d1e0ba 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -71,7 +71,8 @@ def test_asyncio_server_start_serving(app): assert srv.is_serving() is False loop.run_until_complete(srv.start_serving()) assert srv.is_serving() is True - srv.close() + wait_close = srv.close() + loop.run_until_complete(wait_close) # Looks like we can't easily test `serve_forever()` def test_app_loop_not_running(app): diff --git a/tests/test_asgi.py b/tests/test_asgi.py index 2bf7edb1..21036652 100644 --- a/tests/test_asgi.py +++ b/tests/test_asgi.py @@ -1,4 +1,5 @@ import asyncio +import sys from collections import deque, namedtuple @@ -81,7 +82,12 @@ def test_listeners_triggered(app): with pytest.warns(UserWarning): server.run() - for task in asyncio.Task.all_tasks(): + all_tasks = ( + asyncio.Task.all_tasks() + if sys.version_info < (3, 7) else + asyncio.all_tasks(asyncio.get_event_loop()) + ) + for task in all_tasks: task.cancel() assert before_server_start @@ -126,7 +132,12 @@ def test_listeners_triggered_async(app): with pytest.warns(UserWarning): server.run() - for task in asyncio.Task.all_tasks(): + all_tasks = ( + asyncio.Task.all_tasks() + if sys.version_info < (3, 7) else + asyncio.all_tasks(asyncio.get_event_loop()) + ) + for task in all_tasks: task.cancel() assert before_server_start diff --git a/tests/test_cookies.py b/tests/test_cookies.py index 74e894c4..1c29c551 100644 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -54,7 +54,7 @@ def test_false_cookies_encoded(app, httponly, expected): response = text("hello cookies") response.cookies["hello"] = "world" response.cookies["hello"]["httponly"] = httponly - return text(response.cookies["hello"].encode("utf8")) + return text(response.cookies["hello"].encode("utf8").decode()) request, response = app.test_client.get("/") diff --git a/tests/test_create_task.py b/tests/test_create_task.py index 4ea5c845..e128263b 100644 --- a/tests/test_create_task.py +++ b/tests/test_create_task.py @@ -17,12 +17,12 @@ def test_create_task(app): @app.route("/early") def not_set(request): - return text(e.is_set()) + return text(str(e.is_set())) @app.route("/late") async def set(request): await asyncio.sleep(0.1) - return text(e.is_set()) + return text(str(e.is_set())) request, response = app.test_client.get("/early") assert response.body == b"False" diff --git a/tests/test_requests.py b/tests/test_requests.py index a405eebc..12c06254 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -1607,33 +1607,6 @@ async def test_request_args_no_query_string_await(app): assert request.args == {} -def test_request_raw_args(app): - - params = {"test": "OK"} - - @app.get("/") - def handler(request): - return text("pass") - - request, response = app.test_client.get("/", params=params) - - assert request.raw_args == params - - -@pytest.mark.asyncio -async def test_request_raw_args_asgi(app): - - params = {"test": "OK"} - - @app.get("/") - def handler(request): - return text("pass") - - request, response = await app.asgi_client.get("/", params=params) - - assert request.raw_args == params - - def test_request_query_args(app): # test multiple params with the same key params = [("test", "value1"), ("test", "value2")] diff --git a/tests/test_response.py b/tests/test_response.py index acb715db..fc2dab07 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -30,6 +30,7 @@ from sanic.testing import HOST, PORT JSON_DATA = {"ok": True} +@pytest.mark.filterwarnings("ignore:Types other than str will be") def test_response_body_not_a_string(app): """Test when a response body sent from the application is not a string""" random_num = choice(range(1000)) diff --git a/tests/test_routes.py b/tests/test_routes.py index 7db136fd..961dba1c 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -770,55 +770,6 @@ def test_add_route_method_not_allowed(app): assert response.status == 405 -def test_remove_static_route(app): - async def handler1(request): - return text("OK1") - - async def handler2(request): - return text("OK2") - - app.add_route(handler1, "/test") - app.add_route(handler2, "/test2") - - request, response = app.test_client.get("/test") - assert response.status == 200 - - request, response = app.test_client.get("/test2") - assert response.status == 200 - - app.remove_route("/test") - app.remove_route("/test2") - - request, response = app.test_client.get("/test") - assert response.status == 404 - - request, response = app.test_client.get("/test2") - assert response.status == 404 - - -def test_remove_dynamic_route(app): - async def handler(request, name): - return text("OK") - - app.add_route(handler, "/folder/") - - request, response = app.test_client.get("/folder/test123") - assert response.status == 200 - - app.remove_route("/folder/") - request, response = app.test_client.get("/folder/test123") - assert response.status == 404 - - -def test_remove_inexistent_route(app): - - uri = "/test" - with pytest.raises(RouteDoesNotExist) as excinfo: - app.remove_route(uri) - - assert str(excinfo.value) == f"Route was not registered: {uri}" - - def test_removing_slash(app): @app.get("/rest/") def get(_): @@ -831,59 +782,6 @@ def test_removing_slash(app): assert len(app.router.routes_all.keys()) == 2 -def test_remove_unhashable_route(app): - async def handler(request, unhashable): - return text("OK") - - app.add_route(handler, "/folder//end/") - - request, response = app.test_client.get("/folder/test/asdf/end/") - assert response.status == 200 - - request, response = app.test_client.get("/folder/test///////end/") - assert response.status == 200 - - request, response = app.test_client.get("/folder/test/end/") - assert response.status == 200 - - app.remove_route("/folder//end/") - - request, response = app.test_client.get("/folder/test/asdf/end/") - assert response.status == 404 - - request, response = app.test_client.get("/folder/test///////end/") - assert response.status == 404 - - request, response = app.test_client.get("/folder/test/end/") - assert response.status == 404 - - -def test_remove_route_without_clean_cache(app): - async def handler(request): - return text("OK") - - app.add_route(handler, "/test") - - request, response = app.test_client.get("/test") - assert response.status == 200 - - app.remove_route("/test", clean_cache=True) - app.remove_route("/test/", clean_cache=True) - - request, response = app.test_client.get("/test") - assert response.status == 404 - - app.add_route(handler, "/test") - - request, response = app.test_client.get("/test") - assert response.status == 200 - - app.remove_route("/test", clean_cache=False) - - request, response = app.test_client.get("/test") - assert response.status == 200 - - def test_overload_routes(app): @app.route("/overload", methods=["GET"]) async def handler1(request): diff --git a/tests/test_static.py b/tests/test_static.py index 984e22c4..711513eb 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -376,17 +376,3 @@ def test_static_name(app, static_file_directory, static_name, file_name): request, response = app.test_client.get(f"/static/{file_name}") assert response.status == 200 - - -@pytest.mark.parametrize("file_name", ["test.file"]) -def test_static_remove_route(app, static_file_directory, file_name): - app.static( - "/testing.file", get_file_path(static_file_directory, file_name) - ) - - request, response = app.test_client.get("/testing.file") - assert response.status == 200 - - app.remove_route("/testing.file") - request, response = app.test_client.get("/testing.file") - assert response.status == 404 diff --git a/tests/test_vhosts.py b/tests/test_vhosts.py index b360b1dc..8b060584 100644 --- a/tests/test_vhosts.py +++ b/tests/test_vhosts.py @@ -48,17 +48,3 @@ def test_vhosts_with_defaults(app): request, response = app.test_client.get("/") assert response.text == "default" - - -def test_remove_vhost_route(app): - @app.route("/", host="example.com") - async def handler1(request): - return text("You're at example.com!") - - headers = {"Host": "example.com"} - request, response = app.test_client.get("/", headers=headers) - assert response.status == 200 - - app.remove_route("/", host="example.com") - request, response = app.test_client.get("/", headers=headers) - assert response.status == 404 diff --git a/tests/test_worker.py b/tests/test_worker.py index 3e83fa13..d8726686 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -128,6 +128,8 @@ def test_handle_quit(worker): assert not worker.alive assert worker.exit_code == 0 +async def _a_noop(*a, **kw): + pass def test_run_max_requests_exceeded(worker): loop = asyncio.new_event_loop() @@ -145,7 +147,7 @@ def test_run_max_requests_exceeded(worker): "server2": {"requests_count": 15}, } worker.max_requests = 10 - worker._run = mock.Mock(wraps=asyncio.coroutine(lambda *a, **kw: None)) + worker._run = mock.Mock(wraps=_a_noop) # exceeding request count _runner = asyncio.ensure_future(worker._check_alive(), loop=loop) @@ -160,7 +162,7 @@ def test_run_max_requests_exceeded(worker): def test_worker_close(worker): loop = asyncio.new_event_loop() - asyncio.sleep = mock.Mock(wraps=asyncio.coroutine(lambda *a, **kw: None)) + asyncio.sleep = mock.Mock(wraps=_a_noop) worker.ppid = 1 worker.pid = 2 worker.cfg.graceful_timeout = 1.0 @@ -169,17 +171,13 @@ def test_worker_close(worker): worker.wsgi = mock.Mock() conn = mock.Mock() conn.websocket = mock.Mock() - conn.websocket.close_connection = mock.Mock( - wraps=asyncio.coroutine(lambda *a, **kw: None) - ) + conn.websocket.close_connection = mock.Mock(wraps=_a_noop) worker.connections = set([conn]) worker.log = mock.Mock() worker.loop = loop server = mock.Mock() server.close = mock.Mock(wraps=lambda *a, **kw: None) - server.wait_closed = mock.Mock( - wraps=asyncio.coroutine(lambda *a, **kw: None) - ) + server.wait_closed = mock.Mock(wraps=_a_noop) worker.servers = {server: {"requests_count": 14}} worker.max_requests = 10 diff --git a/tox.ini b/tox.ini index 321e3a55..9c9e9dd3 100644 --- a/tox.ini +++ b/tox.ini @@ -9,6 +9,7 @@ setenv = deps = coverage pytest==5.2.1 + pytest-asyncio pytest-cov pytest-sanic pytest-sugar