Deprecation and test cleanup (#1818)

* 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 <tronic@users.noreply.github.com>

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

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,

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:

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.

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)

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):

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

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("/")
 

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"

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")]

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))

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/<name>")
-
-    request, response = app.test_client.get("/folder/test123")
-    assert response.status == 200
-
-    app.remove_route("/folder/<name>")
-    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/<resource>")
     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/<unhashable:[A-Za-z0-9/]+>/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/<unhashable:[A-Za-z0-9/]+>/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):

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

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

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
 

tox.ini

@@ -9,6 +9,7 @@ setenv =
 deps =
     coverage
     pytest==5.2.1
+    pytest-asyncio
     pytest-cov
     pytest-sanic
     pytest-sugar