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 <[email protected]>

sanic/app.py

@@ -507,12 +507,7 @@ async def websocket_handler(request, *args, **kwargs):
                 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 @@ def cancel_websocket_tasks(app, loop):
 
         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 @@ def get_args(
 
     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 @@ def get_headers(
     ):
         """.. 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 @@ def check_dynamic_route_exists(pattern, routes_to_check, parameters):
         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 install_signal_handlers(self):
     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 install_signal_handlers(self):
     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 handler(request):
         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 @@ async def coro():
 
     @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 @@ def handler(request):
     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 @@
 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 @@ async def handler(request):
     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 post(_):
     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 @@ async def handler2(request):
 
     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