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

integration test update for Esmerald #1087

Merged
merged 3 commits into from
Sep 30, 2024
Merged

integration test update for Esmerald #1087

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d99c7ca
integration test update for Esmerald
sinisaos Sep 29, 2024
d38e25e
Merge branch 'piccolo-orm:master' into update_integration_tests
sinisaos Sep 30, 2024
1b6f8b4
use @dantownsend code for dummy_server
sinisaos Sep 30, 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
4 changes: 2 additions & 2 deletions piccolo/apps/asgi/commands/new.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
SERVERS = ["uvicorn", "Hypercorn", "granian"]
ROUTER_DEPENDENCIES = {
"starlette": ["starlette"],
"fastapi": ["fastapi>=0.112.1"],
Copy link
Member

Choose a reason for hiding this comment

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

I think we version pinned this because of a version mismatch with Starlette - presumably this is fixed now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think it's fixed and I had no problems in testing.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, thanks

"fastapi": ["fastapi"],
"blacksheep": ["blacksheep"],
"litestar": ["litestar"],
"esmerald": ["esmerald==3.3.0"],
"esmerald": ["esmerald"],
"lilya": ["lilya"],
}
ROUTERS = list(ROUTER_DEPENDENCIES.keys())
Expand Down
1 change: 1 addition & 0 deletions tests/apps/asgi/commands/files/dummy_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ async def receive():
"method": "GET",
"query_string": b"",
"headers": [],
"_body": b"null",
Copy link
Member

@dantownsend dantownsend Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I looked into this a bit, and you're right - _body seems to be the only thing different in the scopes.

I looked at the Esmerald codebase, and it looks like _body is just an Esmerald thing, and not a generic ASGI thing, so even though it works, I can't really explain why this works.

I had an idea - we could use the httpx test client instead:

import asyncio
import importlib
import sys
import typing as t

from httpx import AsyncClient


async def dummy_server(app: t.Union[str, t.Callable] = "app:app"):
    """
    A very simplistic ASGI server. It's used to run the generated ASGI
    applications in unit tests.

    :param app:
        Either an ASGI app, or a string representing the path to an ASGI app.
        For example, ``module_1.app:app`` which would import an ASGI app called
        ``app`` from ``module_1.app``.

    """
    print("Running dummy server ...")

    if isinstance(app, str):
        path, app_name = app.rsplit(":")
        module = importlib.import_module(path)
        app = getattr(module, app_name)

    async with AsyncClient(app=app) as client:
        response = await client.get('http://localhost:8000')
        if response.status_code != 200:
            sys.exit("The app isn't callable!")


if __name__ == "__main__":
    asyncio.run(dummy_server())

Our existing dummy server must be too simplistic, and isn't doing something.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is great. I tried that and it works great. I just need to add httpx to test-requirements.txtand replace

async def dummy_server(app: t.Union[str, t.Callable] = "app:app"):

with

async def dummy_server(app: t.Union[t.Any, t.Callable] = "app:app"):

to keep mypy happy.

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, I see the mypy error now.

You can also do this:

app = t.cast(t.Callable, getattr(module, app_name))

Instead of setting the arg type to Any.

}
if callable(app):
asyncio.run(app(scope, receive, send))
Expand Down
Loading