Skip to content

Commit

Permalink
Make tests framework agnostic (#1634)
Browse files Browse the repository at this point in the history
Currently, our tests only run against the `FlaskApp`. This PR
parametrizes the relevant tests to run against both the `AsyncApp` and
`FlaskApp`.

I used the Starlette TestClient for both apps. Since both apps are wrapped by ASGI middleware, this works, and we no longer need to write our tests to match both test client interfaces.

This increased the number of tests from 483 to 709.
  • Loading branch information
RobbeSneyders committed Feb 17, 2023
2 parents 642a5f2 + 19bf15e commit 04dcfe1
Show file tree
Hide file tree
Showing 35 changed files with 865 additions and 715 deletions.
3 changes: 1 addition & 2 deletions connexion/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,14 @@
from .utils import not_installed_error # NOQA

try:
from flask import request # NOQA

from connexion.apps.flask import FlaskApi, FlaskApp
except ImportError as e: # pragma: no cover
_flask_not_installed_error = not_installed_error(e)
FlaskApi = _flask_not_installed_error # type: ignore
FlaskApp = _flask_not_installed_error # type: ignore

from connexion.apps.asynchronous import AsyncApi, AsyncApp
from connexion.context import request
from connexion.middleware import ConnexionMiddleware

App = FlaskApp
Expand Down
3 changes: 2 additions & 1 deletion connexion/apps/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import pathlib
import typing as t

from starlette.testclient import TestClient
from starlette.types import Receive, Scope, Send

from connexion.middleware import ConnexionMiddleware, SpecMiddleware
Expand Down Expand Up @@ -221,9 +222,9 @@ def add_error_handler(
:param function: Callable that will handle exception.
"""

@abc.abstractmethod
def test_client(self, **kwargs):
"""Creates a test client for this application."""
return TestClient(self, **kwargs)

def run(self, import_string: str = None, **kwargs):
"""Run the application using uvicorn.
Expand Down
5 changes: 1 addition & 4 deletions connexion/apps/asynchronous.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,4 @@ def add_url_rule(
def add_error_handler(
self, code_or_exception: t.Union[int, t.Type[Exception]], function: t.Callable
) -> None:
"""TODO: implement"""

def test_client(self, **kwargs):
"""TODO: implement"""
self.middleware.add_error_handler(code_or_exception, function)
5 changes: 0 additions & 5 deletions connexion/apps/flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import pathlib
import typing as t

import a2wsgi
import flask
import werkzeug.exceptions
from flask import Response as FlaskResponse
Expand Down Expand Up @@ -250,7 +249,3 @@ def add_error_handler(
self, code_or_exception: t.Union[int, t.Type[Exception]], function: t.Callable
) -> None:
self.app.register_error_handler(code_or_exception, function)

def test_client(self, **kwargs):
self.app.wsgi_app = a2wsgi.ASGIMiddleware(self.middleware)
return self.app.test_client(**kwargs)
5 changes: 5 additions & 0 deletions connexion/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from starlette.types import Receive, Scope
from werkzeug.local import LocalProxy

from connexion.lifecycle import ASGIRequest
from connexion.operations import AbstractOperation

UNBOUND_MESSAGE = (
Expand All @@ -22,3 +23,7 @@

_scope: ContextVar[Scope] = ContextVar("SCOPE")
scope = LocalProxy(_scope, unbound_message=UNBOUND_MESSAGE)

request = LocalProxy(
lambda: ASGIRequest(scope, receive), unbound_message=UNBOUND_MESSAGE
)
15 changes: 9 additions & 6 deletions connexion/decorators/parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from connexion.frameworks.flask import Flask as FlaskFramework
from connexion.frameworks.starlette import Starlette as StarletteFramework
from connexion.http_facts import FORM_CONTENT_TYPES
from connexion.lifecycle import ConnexionRequest, MiddlewareRequest
from connexion.lifecycle import ASGIRequest, WSGIRequest
from connexion.operations import AbstractOperation, Swagger2Operation
from connexion.utils import deep_merge, is_null, is_nullable, make_type

Expand All @@ -37,7 +37,7 @@ def __init__(

def _maybe_get_body(
self,
request: t.Union[ConnexionRequest, MiddlewareRequest],
request: t.Union[WSGIRequest, ASGIRequest],
*,
arguments: t.List[str],
has_kwargs: bool,
Expand Down Expand Up @@ -67,14 +67,15 @@ def __call__(self, function: t.Callable) -> t.Callable:
arguments, has_kwargs = inspect_function_arguments(unwrapped_function)

@functools.wraps(function)
def wrapper(request: ConnexionRequest) -> t.Any:
def wrapper(request: WSGIRequest) -> t.Any:
request_body = self._maybe_get_body(
request, arguments=arguments, has_kwargs=has_kwargs
)

kwargs = prep_kwargs(
request,
request_body=request_body,
files=request.files(),
arguments=arguments,
has_kwargs=has_kwargs,
sanitize=self.sanitize_fn,
Expand All @@ -94,7 +95,7 @@ def __call__(self, function: t.Callable) -> t.Callable:
arguments, has_kwargs = inspect_function_arguments(unwrapped_function)

@functools.wraps(function)
async def wrapper(request: MiddlewareRequest) -> t.Any:
async def wrapper(request: ASGIRequest) -> t.Any:
request_body = self._maybe_get_body(
request, arguments=arguments, has_kwargs=has_kwargs
)
Expand All @@ -105,6 +106,7 @@ async def wrapper(request: MiddlewareRequest) -> t.Any:
kwargs = prep_kwargs(
request,
request_body=request_body,
files=await request.files(),
arguments=arguments,
has_kwargs=has_kwargs,
sanitize=self.sanitize_fn,
Expand All @@ -116,9 +118,10 @@ async def wrapper(request: MiddlewareRequest) -> t.Any:


def prep_kwargs(
request: t.Union[ConnexionRequest, MiddlewareRequest],
request: t.Union[WSGIRequest, ASGIRequest],
*,
request_body: t.Any,
files: t.Dict[str, t.Any],
arguments: t.List[str],
has_kwargs: bool,
sanitize: t.Callable,
Expand All @@ -128,7 +131,7 @@ def prep_kwargs(
path_params=request.path_params,
query_params=request.query_params,
body=request_body,
files=request.files,
files=files,
arguments=arguments,
has_kwargs=has_kwargs,
sanitize=sanitize,
Expand Down
6 changes: 3 additions & 3 deletions connexion/decorators/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from connexion.datastructures import NoContent
from connexion.exceptions import NonConformingResponseHeaders
from connexion.frameworks.abstract import Framework
from connexion.lifecycle import ConnexionResponse, MiddlewareResponse
from connexion.lifecycle import ConnexionResponse
from connexion.utils import is_json_mimetype

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -160,7 +160,7 @@ def wrapper(*args, **kwargs):
handler_response = function(*args, **kwargs)
if self.framework.is_framework_response(handler_response):
return handler_response
elif isinstance(handler_response, (ConnexionResponse, MiddlewareResponse)):
elif isinstance(handler_response, ConnexionResponse):
return self.framework.connexion_to_framework_response(handler_response)
else:
return self.build_framework_response(handler_response)
Expand All @@ -180,7 +180,7 @@ async def wrapper(*args, **kwargs):
handler_response = await function(*args, **kwargs)
if self.framework.is_framework_response(handler_response):
return handler_response
elif isinstance(handler_response, (ConnexionResponse, MiddlewareResponse)):
elif isinstance(handler_response, ConnexionResponse):
return self.framework.connexion_to_framework_response(handler_response)
else:
return self.build_framework_response(handler_response)
Expand Down
8 changes: 5 additions & 3 deletions connexion/frameworks/flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from connexion import jsonifier
from connexion.frameworks.abstract import Framework
from connexion.lifecycle import ConnexionRequest
from connexion.lifecycle import WSGIRequest
from connexion.uri_parsing import AbstractURIParser


Expand Down Expand Up @@ -54,8 +54,10 @@ def build_response(
return flask.current_app.response_class(**kwargs)

@staticmethod
def get_request(*, uri_parser: AbstractURIParser, **kwargs) -> ConnexionRequest: # type: ignore
return ConnexionRequest(flask.request, uri_parser=uri_parser)
def get_request(*, uri_parser: AbstractURIParser, **kwargs) -> WSGIRequest: # type: ignore
return WSGIRequest(
flask.request, uri_parser=uri_parser, view_args=flask.request.view_args
)


PATH_PARAMETER = re.compile(r"\{([^}]*)\}")
Expand Down
11 changes: 5 additions & 6 deletions connexion/frameworks/starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@
from starlette.types import Receive, Scope

from connexion.frameworks.abstract import Framework
from connexion.lifecycle import MiddlewareRequest, MiddlewareResponse
from connexion.lifecycle import ASGIRequest
from connexion.uri_parsing import AbstractURIParser


class Starlette(Framework):
@staticmethod
def is_framework_response(response: t.Any) -> bool:
return isinstance(response, StarletteResponse) and not isinstance(
response, MiddlewareResponse
)
return isinstance(response, StarletteResponse)

@classmethod
def connexion_to_framework_response(cls, response):
Expand Down Expand Up @@ -49,8 +48,8 @@ def build_response(
)

@staticmethod
def get_request(*, scope: Scope, receive: Receive, **kwargs) -> MiddlewareRequest: # type: ignore
return MiddlewareRequest(scope, receive)
def get_request(*, scope: Scope, receive: Receive, uri_parser: AbstractURIParser, **kwargs) -> ASGIRequest: # type: ignore
return ASGIRequest(scope, receive, uri_parser=uri_parser)


PATH_PARAMETER = re.compile(r"\{([^}]*)\}")
Expand Down

0 comments on commit 04dcfe1

Please sign in to comment.