Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(testing): add msgpack support #2394

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

feat(testing): add msgpack support #2394

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9e9e3cd
tried to add support for messagepack
arthurprioli Oct 18, 2024
8deb751
added documentation
arthurprioli Oct 20, 2024
97a5a2a
fixed ruff formatting
arthurprioli Oct 20, 2024
58e251d
Merge branch 'master' into 1026-add-messsagepack-support
arthurprioli Oct 28, 2024
2cea4f6
tests added
arthurprioli Oct 30, 2024
f911793
inserted changes asked by code review
arthurprioli Oct 30, 2024
aa6edf2
fixed overidented docstrings
arthurprioli Oct 30, 2024
37d702e
Merge branch 'master' into 1026-add-messsagepack-support
vytas7 Nov 6, 2024
4db15eb
Merge branch 'master' into 1026-add-messsagepack-support
arthurprioli Nov 10, 2024
918ba30
added more tests, fixed documentation and wrote better client.py
arthurprioli Nov 20, 2024
ef7a39d
Merge branch '1026-add-messsagepack-support' of https://github.com/ar…
arthurprioli Nov 20, 2024
f226b5b
Merge branch 'master' into 1026-add-messsagepack-support
vytas7 Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions docs/_newsfragments/2387.misc.rst
View file
Open in desktop

This file was deleted.

14 changes: 13 additions & 1 deletion docs/changes/4.1.0.rst
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newsfragment file missing. You don't need to manually include it in 4.1.0.rst, but rather create a separate newsfragment file. Please check our docs how to contribute these files.

Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,21 @@ Changes to Supported Platforms

.. NOTE(vytas): No changes to the supported platforms (yet).


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A diff still remains in this file against master. Please revert all your changes to this release template.

.. towncrier release notes start


Misc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not render out newsfragments yourself; a Falcon release manager does that when the time comes.
Moreover, this fragment looks stale, it is already release as part of Falcon 4.0.2.

----

- Running mypy on code that uses parts of ``falcon.testing`` naively
would lead to errors like::

Name "falcon.testing.TestClient" is not defined

This has been fixed by explicitly exporting the names that are
imported in the ``falcon.testing`` namespace. (`#2387 <https://github.com/falconry/falcon/issues/2387>`__)


Contributors to this Release
----------------------------

Expand Down
40 changes: 40 additions & 0 deletions falcon/testing/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@
from falcon.asgi_spec import ScopeType
from falcon.constants import COMBINED_METHODS
from falcon.constants import MEDIA_JSON
from falcon.constants import MEDIA_MSGPACK
from falcon.errors import CompatibilityError
from falcon.media import MessagePackHandler
from falcon.testing import helpers
from falcon.testing.srmock import StartResponseMock
from falcon.typing import Headers
Expand Down Expand Up @@ -455,6 +457,7 @@ def simulate_request(
content_type: Optional[str] = None,
body: Optional[Union[str, bytes]] = None,
json: Optional[Any] = None,
msgpack: Optional[Any] = None,
file_wrapper: Optional[Callable[..., Any]] = None,
wsgierrors: Optional[TextIO] = None,
params: Optional[Mapping[str, Any]] = None,
Expand Down Expand Up @@ -592,6 +595,7 @@ def simulate_request(
content_type=content_type,
body=body,
json=json,
msgpack=msgpack,
params=params,
params_csv=params_csv,
protocol=protocol,
Expand All @@ -615,6 +619,7 @@ def simulate_request(
headers,
body,
json,
msgpack,
extras,
)

Expand Down Expand Up @@ -724,6 +729,7 @@ async def _simulate_request_asgi(
content_type: Optional[str] = None,
body: Optional[Union[str, bytes]] = None,
json: Optional[Any] = None,
msgpack: Optional[Any] = None,
params: Optional[Mapping[str, Any]] = None,
params_csv: bool = False,
protocol: str = 'http',
Expand Down Expand Up @@ -808,6 +814,11 @@ async def _simulate_request_asgi(
overrides `body` and sets the Content-Type header to
``'application/json'``, overriding any value specified by either
the `content_type` or `headers` arguments.
msgpack(Msgpack serializable): A Msgpack document to serialize as the
arthurprioli marked this conversation as resolved.
Show resolved Hide resolved
body of the request (default: ``None``). If specified,
overrides `body` and sets the Content-Type header to
``'application/msgpack'``, overriding any value specified by
either the `content_type` or `headers` arguments.
host(str): A string to use for the hostname part of the fully
qualified request URL (default: 'falconframework.org')
remote_addr (str): A string to use as the remote IP address for the
Expand Down Expand Up @@ -846,6 +857,7 @@ async def _simulate_request_asgi(
headers,
body,
json,
msgpack,
extras,
)

Expand Down Expand Up @@ -1551,6 +1563,11 @@ def simulate_post(app: Callable[..., Any], path: str, **kwargs: Any) -> Result:
overrides `body` and sets the Content-Type header to
``'application/json'``, overriding any value specified by either
the `content_type` or `headers` arguments.
msgpack(Msgpack serializable): A Msgpack document to serialize as the
body of the request (default: ``None``). If specified,
overrides `body` and sets the Content-Type header to
``'application/msgpack'``, overriding any value specified by
either the `content_type` or `headers` arguments.
file_wrapper (callable): Callable that returns an iterable,
to be used as the value for *wsgi.file_wrapper* in the
WSGI environ (default: ``None``). This can be used to test
Expand Down Expand Up @@ -1662,6 +1679,11 @@ def simulate_put(app: Callable[..., Any], path: str, **kwargs: Any) -> Result:
overrides `body` and sets the Content-Type header to
``'application/json'``, overriding any value specified by either
the `content_type` or `headers` arguments.
msgpack(Msgpack serializable): A Msgpack document to serialize as the
body of the request (default: ``None``). If specified,
overrides `body` and sets the Content-Type header to
``'application/msgpack'``, overriding any value specified by
either the `content_type` or `headers` arguments.
file_wrapper (callable): Callable that returns an iterable,
to be used as the value for *wsgi.file_wrapper* in the
WSGI environ (default: ``None``). This can be used to test
Expand Down Expand Up @@ -1862,6 +1884,11 @@ def simulate_patch(app: Callable[..., Any], path: str, **kwargs: Any) -> Result:
overrides `body` and sets the Content-Type header to
``'application/json'``, overriding any value specified by either
the `content_type` or `headers` arguments.
msgpack(Msgpack serializable): A Msgpack document to serialize as the
body of the request (default: ``None``). If specified,
overrides `body` and sets the Content-Type header to
``'application/msgpack'``, overriding any value specified by
either the `content_type` or `headers` arguments.
host(str): A string to use for the hostname part of the fully
qualified request URL (default: 'falconframework.org')
remote_addr (str): A string to use as the remote IP address for the
Expand Down Expand Up @@ -1968,6 +1995,11 @@ def simulate_delete(app: Callable[..., Any], path: str, **kwargs: Any) -> Result
overrides `body` and sets the Content-Type header to
``'application/json'``, overriding any value specified by either
the `content_type` or `headers` arguments.
msgpack(Msgpack serializable): A Msgpack document to serialize as the
body of the request (default: ``None``). If specified,
overrides `body` and sets the Content-Type header to
``'application/msgpack'``, overriding any value specified by
either the `content_type` or `headers` arguments.
host(str): A string to use for the hostname part of the fully
qualified request URL (default: 'falconframework.org')
remote_addr (str): A string to use as the remote IP address for the
Expand Down Expand Up @@ -2248,6 +2280,7 @@ def _prepare_sim_args(
headers: Optional[HeaderArg],
body: Optional[Union[str, bytes]],
json: Optional[Any],
msgpack: Optional[Any],
extras: Optional[Mapping[str, Any]],
) -> Tuple[
str, str, Optional[HeaderArg], Optional[Union[str, bytes]], Mapping[str, Any]
Expand Down Expand Up @@ -2284,6 +2317,13 @@ def _prepare_sim_args(
headers = dict(headers or {})
headers['Content-Type'] = MEDIA_JSON

if msgpack is not None:
body = MessagePackHandler.serialize(
MessagePackHandler(), content_type=None, media=msgpack
arthurprioli marked this conversation as resolved.
Show resolved Hide resolved
)
headers = headers or {}
headers['Content-Type'] = MEDIA_MSGPACK

return path, query_string, headers, body, extras


Expand Down
14 changes: 14 additions & 0 deletions tests/test_testing.py
View file
Open in desktop
Copy link
Member

@vytas7 vytas7 Nov 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need more extensive tests for this new feature.
You have a couple of test cases there, but we should also check whether the simulated body is correct, not just the content type.

I would suggest to use pytest.mark.parametrize(...) to create multiple test cases from different test data, but the same test code.

We also want to test the combination of msgpack= together with other parameters such as json=, etc, in order to verify that the documented precedence order is correct.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a doubt about that, with json we pass parameters (json), (json, headers), (json,content-type), (json, headers, content-type), I did similar tests with both msgpack and json with msgpack, but mintests still breaks. What's the best way to parametrize it? And how do I check if simulated body is correct?

Copy link
Member

@vytas7 vytas7 Nov 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to add/adapt tests checking what happens when msgpack is not installed.
Judging from the tox -e mintest output, this case is not always handled as expected in the proposed changeset, at least not in the tests.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would I do that? Would it be with a try, except statement while importing msgpack? Or is there a better way for testing it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check other existing tests, e.g., tests/test_media_multipart.py, or many others.

You can reuse the following pattern:

# Somewhere in the beginning of the file, towards the end of the import blocks:
try:
    import msgpack
except ImportError:
    msgpack = None

Then, later, shield your tests in question with the pytest.mark.skipif(...) decorator, e.g.:

@pytest.mark.skipif(msgpack is None, reason='msgpack is required for this test')
def test_my_new_msgpack_parameter(some_param, ...):
    ...

Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,20 @@ def on_post(self, req, resp):
)
assert result.text == falcon.MEDIA_JSON

result = testing.simulate_post(app, '/', msgpack={})
assert result.text == falcon.MEDIA_MSGPACK

result = testing.simulate_post(app, '/', msgpack={}, content_type=falcon.MEDIA_HTML)
assert result.text == falcon.MEDIA_MSGPACK

result = testing.simulate_post(app, '/', msgpack={}, headers=headers)
assert result.text == falcon.MEDIA_MSGPACK

result = testing.simulate_post(
app, '/', msgpack={}, headers=headers, content_type=falcon.MEDIA_HTML
)
assert result.text == falcon.MEDIA_MSGPACK
arthurprioli marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.parametrize('mode', ['wsgi', 'asgi', 'asgi-stream'])
def test_content_type(util, mode):
Expand Down
Loading