From 34fe26e51bc13cd41d58e627ba264012640c76fc Mon Sep 17 00:00:00 2001 From: Harsha Narayana Date: Thu, 28 Feb 2019 20:26:41 +0530 Subject: [PATCH] Add Route Resolution Benchmarking to Unit Test (#1499) * feat: add benchmark tester for route resolution and cleanup test warnings Signed-off-by: Harsha Narayana * feat: refactor sanic benchmark test util into fixtures Signed-off-by: Harsha Narayana --- .../test_route_resolution_benchmark.py | 53 ++++++++ tests/conftest.py | 118 ++++++++++++++++++ tests/test_app.py | 5 +- tests/test_config.py | 7 +- tests/test_cookies.py | 2 +- tests/test_dynamic_routes.py | 2 +- tests/test_exceptions.py | 4 +- tests/test_request_stream.py | 5 +- tests/test_requests.py | 2 +- tests/test_url_building.py | 4 +- tox.ini | 5 + 11 files changed, 192 insertions(+), 15 deletions(-) create mode 100644 tests/benchmark/test_route_resolution_benchmark.py diff --git a/tests/benchmark/test_route_resolution_benchmark.py b/tests/benchmark/test_route_resolution_benchmark.py new file mode 100644 index 00000000..d0df69a1 --- /dev/null +++ b/tests/benchmark/test_route_resolution_benchmark.py @@ -0,0 +1,53 @@ +from random import choice, seed +from pytest import mark + +import sanic.router + +seed("Pack my box with five dozen liquor jugs.") + +# Disable Caching for testing purpose +sanic.router.ROUTER_CACHE_SIZE = 0 + + +class TestSanicRouteResolution: + @mark.asyncio + async def test_resolve_route_no_arg_string_path( + self, sanic_router, route_generator, benchmark + ): + simple_routes = route_generator.generate_random_direct_route( + max_route_depth=4 + ) + router, simple_routes = sanic_router(route_details=simple_routes) + route_to_call = choice(simple_routes) + + result = benchmark.pedantic( + router._get, + ("/{}".format(route_to_call[-1]), route_to_call[0], "localhost"), + iterations=1000, + rounds=1000, + ) + assert await result[0](None) == 1 + + @mark.asyncio + async def test_resolve_route_with_typed_args( + self, sanic_router, route_generator, benchmark + ): + typed_routes = route_generator.add_typed_parameters( + route_generator.generate_random_direct_route(max_route_depth=4), + max_route_depth=8, + ) + router, typed_routes = sanic_router(route_details=typed_routes) + route_to_call = choice(typed_routes) + url = route_generator.generate_url_for_template( + template=route_to_call[-1] + ) + + print("{} -> {}".format(route_to_call[-1], url)) + + result = benchmark.pedantic( + router._get, + ("/{}".format(url), route_to_call[0], "localhost"), + iterations=1000, + rounds=1000, + ) + assert await result[0](None) == 1 diff --git a/tests/conftest.py b/tests/conftest.py index 2bab4890..1748b46c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,12 +1,130 @@ +import random +import re +import string import sys +import uuid + import pytest from sanic import Sanic +from sanic.router import RouteExists, Router + +random.seed("Pack my box with five dozen liquor jugs.") if sys.platform in ["win32", "cygwin"]: collect_ignore = ["test_worker.py"] +async def _handler(request): + """ + Dummy placeholder method used for route resolver when creating a new + route into the sanic router. This router is not actually called by the + sanic app. So do not worry about the arguments to this method. + + If you change the return value of this method, make sure to propagate the + change to any test case that leverages RouteStringGenerator. + """ + return 1 + + +TYPE_TO_GENERATOR_MAP = { + "string": lambda: "".join( + [random.choice(string.ascii_letters + string.digits) for _ in range(4)] + ), + "int": lambda: random.choice(range(1000000)), + "number": lambda: random.random(), + "alpha": lambda: "".join( + [random.choice(string.ascii_letters) for _ in range(4)] + ), + "uuid": lambda: str(uuid.uuid1()), +} + + +class RouteStringGenerator: + + ROUTE_COUNT_PER_DEPTH = 100 + HTTP_METHODS = ["GET", "PUT", "POST", "PATCH", "DELETE", "OPTION"] + ROUTE_PARAM_TYPES = ["string", "int", "number", "alpha", "uuid"] + + def generate_random_direct_route(self, max_route_depth=4): + routes = [] + for depth in range(1, max_route_depth + 1): + for _ in range(self.ROUTE_COUNT_PER_DEPTH): + route = "/".join( + [ + TYPE_TO_GENERATOR_MAP.get("string")() + for _ in range(depth) + ] + ) + route = route.replace(".", "", -1) + route_detail = (random.choice(self.HTTP_METHODS), route) + + if route_detail not in routes: + routes.append(route_detail) + return routes + + def add_typed_parameters(self, current_routes, max_route_depth=8): + routes = [] + for method, route in current_routes: + current_length = len(route.split("/")) + new_route_part = "/".join( + [ + "<{}:{}>".format( + TYPE_TO_GENERATOR_MAP.get("string")(), + random.choice(self.ROUTE_PARAM_TYPES), + ) + for _ in range(max_route_depth - current_length) + ] + ) + route = "/".join([route, new_route_part]) + route = route.replace(".", "", -1) + routes.append((method, route)) + return routes + + @staticmethod + def generate_url_for_template(template): + url = template + for pattern, param_type in re.findall( + re.compile(r"((?:<\w+:(string|int|number|alpha|uuid)>)+)"), + template, + ): + value = TYPE_TO_GENERATOR_MAP.get(param_type)() + url = url.replace(pattern, str(value), -1) + return url + + +@pytest.fixture(scope="function") +def sanic_router(): + # noinspection PyProtectedMember + def _setup(route_details: tuple) -> (Router, tuple): + router = Router() + added_router = [] + for method, route in route_details: + try: + router._add( + uri="/{}".format(route), + methods=frozenset({method}), + host="localhost", + handler=_handler, + ) + added_router.append((method, route)) + except RouteExists: + pass + return router, added_router + + return _setup + + +@pytest.fixture(scope="function") +def route_generator() -> RouteStringGenerator: + return RouteStringGenerator() + + +@pytest.fixture(scope="function") +def url_param_generator(): + return TYPE_TO_GENERATOR_MAP + + @pytest.fixture def app(request): return Sanic(request.node.name) diff --git a/tests/test_app.py b/tests/test_app.py index 1003d727..8d90641f 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -56,7 +56,7 @@ def test_asyncio_server_start_serving(app): def test_app_loop_not_running(app): with pytest.raises(SanicException) as excinfo: - app.loop + _ = app.loop assert str(excinfo.value) == ( "Loop can only be retrieved after the app has started " @@ -140,7 +140,6 @@ def test_handle_request_with_nested_exception(app, monkeypatch): @app.get("/") def handler(request): raise Exception - return text("OK") request, response = app.test_client.get("/") assert response.status == 500 @@ -162,7 +161,6 @@ def test_handle_request_with_nested_exception_debug(app, monkeypatch): @app.get("/") def handler(request): raise Exception - return text("OK") request, response = app.test_client.get("/", debug=True) assert response.status == 500 @@ -186,7 +184,6 @@ def test_handle_request_with_nested_sanic_exception(app, monkeypatch, caplog): @app.get("/") def handler(request): raise Exception - return text("OK") with caplog.at_level(logging.ERROR): request, response = app.test_client.get("/") diff --git a/tests/test_config.py b/tests/test_config.py index 0bd5007b..c2da4bdd 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -144,9 +144,10 @@ def test_overwrite_exisiting_config_ignore_lowercase(app): def test_missing_config(app): - with pytest.raises(AttributeError) as e: - app.config.NON_EXISTENT - assert str(e.value) == ("Config has no 'NON_EXISTENT'") + with pytest.raises( + AttributeError, match="Config has no 'NON_EXISTENT'" + ) as e: + _ = app.config.NON_EXISTENT def test_config_defaults(): diff --git a/tests/test_cookies.py b/tests/test_cookies.py index af7858a8..b573b845 100644 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -100,7 +100,7 @@ def test_cookie_deletion(app): assert int(response_cookies["i_want_to_die"]["max-age"]) == 0 with pytest.raises(KeyError): - response.cookies["i_never_existed"] + _ = response.cookies["i_never_existed"] def test_cookie_reserved_cookie(): diff --git a/tests/test_dynamic_routes.py b/tests/test_dynamic_routes.py index e7b3729f..6a5c57c6 100644 --- a/tests/test_dynamic_routes.py +++ b/tests/test_dynamic_routes.py @@ -39,5 +39,5 @@ def test_overload_dynamic_routes_exist(app): with pytest.raises(RouteExists): @app.route("/overload/", methods=["PUT", "DELETE"]) - async def handler3(request): + async def handler3(request, param): return text("Duplicated") diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 5a2f0f8f..a02a7064 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -74,7 +74,7 @@ def exception_app(): @app.route("/divide_by_zero") def handle_unhandled_exception(request): - 1 / 0 + _ = 1 / 0 @app.route("/error_in_error_handler_handler") def custom_error_handler(request): @@ -82,7 +82,7 @@ def exception_app(): @app.exception(SanicExceptionTestException) def error_in_error_handler_handler(request, exception): - 1 / 0 + _ = 1 / 0 return app diff --git a/tests/test_request_stream.py b/tests/test_request_stream.py index 42f2852c..c8978608 100644 --- a/tests/test_request_stream.py +++ b/tests/test_request_stream.py @@ -279,9 +279,12 @@ def test_request_stream_blueprint(app): if body is None: break await response.write(body.decode("utf-8")) + return stream(streaming) - bp.add_route(post_add_route, '/post/add_route', methods=['POST'], stream=True) + bp.add_route( + post_add_route, "/post/add_route", methods=["POST"], stream=True + ) app.blueprint(bp) assert app.is_request_stream is True diff --git a/tests/test_requests.py b/tests/test_requests.py index a98a992e..e10c4538 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -134,7 +134,7 @@ def test_query_string(app): def test_uri_template(app): @app.route("/foo//bar/") - async def handler(request): + async def handler(request, id, name): return text("OK") request, response = app.test_client.get("/foo/123/bar/baz") diff --git a/tests/test_url_building.py b/tests/test_url_building.py index d752c8a3..5e1c300a 100644 --- a/tests/test_url_building.py +++ b/tests/test_url_building.py @@ -169,8 +169,8 @@ def test_fails_with_int_message(app): app.url_for("fail", **failing_kwargs) expected_error = ( - 'Value "not_int" for parameter `foo` ' - "does not match pattern for type `int`: -?\d+" + r'Value "not_int" for parameter `foo` ' + r'does not match pattern for type `int`: -?\d+' ) assert str(e.value) == expected_error diff --git a/tox.ini b/tox.ini index 503d8aa5..502eea81 100644 --- a/tox.ini +++ b/tox.ini @@ -16,6 +16,7 @@ deps = chardet<=2.3.0 beautifulsoup4 gunicorn + pytest-benchmark commands = pytest tests --cov sanic --cov-report= {posargs} - coverage combine --append @@ -39,3 +40,7 @@ deps = pygments commands = python setup.py check -r -s + +[pytest] +filterwarnings = + ignore:.*async with lock.* instead:DeprecationWarning