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.

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

HTTPX-based TestClient? #652

Closed
florimondmanca opened this issue Oct 1, 2019 · 14 comments · Fixed by #1376
Closed

HTTPX-based TestClient? #652

florimondmanca opened this issue Oct 1, 2019 · 14 comments · Fixed by #1376
Labels
testclient TestClient-related

Comments

@florimondmanca
Copy link
Member

Hi folks,

Currently, the TestClient is built as a subclass of requests.Session.

What would be thoughts on migrating to HTTPX?

  • Is this a good idea at all?
  • Should the TestClient be built around HTTPX, or should it eventually be replaced by HTTPX and its ASGI support?

As of today, there are definitely blockers to a full migration. An important one is that HTTPX doesn't support WebSocket yet (see encode/httpx#304), while TestClient does.

Happy to discuss! cc @tiangolo @gvbgduh @tomchristie

@tiangolo
Copy link
Member

tiangolo commented Oct 1, 2019

I'm pretty sure that's part of the initial objective, to move to HTTPX instead of Requests.

Given it's basically the same interface as Requests, I don't think there would be any problem with it for users/developers.

I'm personally happy with it. 🚀

Of course, after solving the blockers, but I think it's a good idea. It should also give more flexibility and control on how to do stuff, how to extend, etc.

I guess we should support the TestClient, probably as a wrapper, at least for some time, to not break existing implementations, even if the decision was to drop the TestClient completely to fully move to HTTPX and and mark the TestClient as deprecated in favor of HTTPX.

I also think we should start adding "how to use HTTPX" to docs, as it's the perfect match for async frameworks when communicating to third parties. But that's another topic.

@dmontagu you have been helping a looot with FastAPI recently ( 🙇‍♂️ 👏 ), do you see any obvious drawbacks to this?

@dmontagu
Copy link
Contributor

dmontagu commented Oct 1, 2019

I think websocket support is an important part of the TestClient, so I think any migration needs a story around that (as @florimondmanca noted). I'm not sure whether websocket support would be in scope for httpx or not.

@tiangolo The biggest drawback I see that I don't think has been mentioned here is that migrating to the httpx client would also require special handling for lifespan events (discussed in encode/httpx#350).

The starlette TestClient currently has special methods that call these lifespan events when entered as a context manager, but the httpx client doesn't support this (and @tomchristie has explicitly said he doesn't think httpx should.)

If starlette replaced the current TestClient with a wrapper around the sync and async httpx clients that added lifespan and websocket handling (and raised an import error if you tried to use it without httpx installed), that would probably be ideal for me.

(And to clear, I think this would have substantial benefits, since it would enable the use of an async test client.)


@florimondmanca Has put together a dependency (asgi-lifespan) that solves the lifespan issues. This is much appreciated 🙌, but longer term I personally would prefer to not need to install a third (admittedly smaller) dependency on top of starlette and httpx to get the current behavior.

I wouldn't mind if either starlette or httpx incorporated asgi-lifespan as a dependency themselves though.

@florimondmanca
Copy link
Member Author

If starlette replaced the current TestClient with a wrapper around the sync and async httpx clients that added lifespan and websocket handling (and raised an import error if you tried to use it without httpx installed), that would probably be ideal for me.

Sounds like a great plan. :)

I'll add to this that the even more ideal situation IMO would be to turn the test client into a dedicated, isolated Python package — similar to async-asgi-testclient.

In fact, slowly building up a reusable ASGI test client as a separate project, with the end goal of matching Starlette's TestClient abilities and telling users to switch is probably a good migration strategy? Bonus point: users that only need HTTP + Lifespan could migrate early on, resulting in a tighter feedback loop?

I wouldn't mind if either starlette or httpx incorporated asgi-lifespan as a dependency themselves though.

That's probably the most sensible approach for Starlette, yes! (But as you noted, lifespan support is out of scope for HTTPX.)

@dmontagu
Copy link
Contributor

dmontagu commented Oct 1, 2019

Yeah, for what it's worth I would be happier to make use of a dedicated asgi test client package than to rely on other packages to patch necessary functionality into httpx just so I could use it as a test client. But I probably wouldn't want to rely on another dependency unless it had substantial traction -- that's one of the benefits of starlette/httpx.

In addition, httpx may be a natural server-side dependency anyway if one wants to make use of webhooks. So I'm sort of on the fence here.

(Either way, I think adding async functionality to the de facto "standard" TestClient, whatever it is, is called for. So something should change 🙂, I just don't have a confident opinion of what it should be.)

@florimondmanca
Copy link
Member Author

florimondmanca commented Oct 1, 2019

But I probably wouldn't want to use it unless it had substantial traction -- that's one of the benefits of starlette/httpx.

Yeah, I see. Actually, keeping it in Starlette might be an okay a desirable situation — especially since Starlette has no dependencies on its own.

Still, I think building up a separate package that eventually lands into Starlette is a good way to quickly iterate without having to release the new test client as part of Starlette, with all the backwards compatibility / risks that come with it. Early adopters can easily switch to importing from Starlette once the test client is mature enough, I guess?

@tiangolo
Copy link
Member

tiangolo commented Oct 1, 2019

Thanks for all the input @dmontagu ! Impecable as always! 🙇‍♂️ 👏

@gvbgduh
Copy link
Member

gvbgduh commented Oct 2, 2019

The whole idea sounds great! I don't have a clear understanding though of how it can be "assembled" and where related bits can be located.

But I would like to highlight an async test client and the lifespan support for it.

The async test client that would use the provided event loop instead of spawning its own one, like in a couple with pytest-asyncio. So, as an example, I'm able to check what was landed in the DB with databases within the test body.

And lifespan is good to test the overall app that has events use like on startup and on shutdown, so you can have close to the real thing experience in testing.

I'm currently using a hand made one in my projects based on requests-async, but it's obviously getting out of date.

@tomchristie
Copy link
Member

Short: yes

  • We should pull in a LifespanManager class directly into Starlette, and document how to use it to ensure proper app setup/shutdown around test cases.
  • We should document how to use httpx as the test client. (Having starlette.TestClient be a synonm to httpx.Client is also either possible, or possible during a deprecation path)
  • This also highlights to me that we might want to assess if we'd like a minimal dependency httpx (eg. h11 by default, with httpx[h2] if you want HTTP/2 support.) I'm thinking along similar lines for uvicorn (h11 by default, with uvloop, httptools, eventually h2 as optionals). We're very strict on dependencies in Starlette, and I think it'd be good to also push for minimal defaults in other encode projects wherever possible.
  • Not sure how websockets fits in - would need more looking into.

@Congee
Copy link

Congee commented Dec 27, 2019

For those who use FastAPI and would like to use httpx instead of requests for testing, I have stolen and adapted some code from our awesome @tomchristie

#!/usr/bin/env python3

# Took credit from @tomchristie
# https://github.com/encode/hostedapi/blob/master/tests/conftest.py

import asyncio

import pytest
from starlette.config import environ

from app.main import app
from tests.client import TestClient

# This sets `os.environ`, but provides some additional protection.
# If we placed it below the application import, it would raise an error
# informing us that 'TESTING' had already been read from the environment.
environ["TESTING"] = "True"


# According to pytest-asyncio README, we should create a new event loop fixture
# to avoid using the default one.
# No. I have to use the default event loop. Because I do not know why our event
# loop is always closed early.
# https://github.com/pytest-dev/pytest-asyncio/blob/86cd9a6fd2/pytest_asyncio/plugin.py#L169-L174
@pytest.fixture(scope='module')
def event_loop():
    # loop = asyncio.get_event_loop_policy().new_event_loop()
    # asyncio.set_event_loop(loop)
    yield asyncio.get_event_loop()
    # assert loop.is_running()


@pytest.fixture(scope='module')
async def client():
    """
    When using the 'client' fixture in test cases, we'll get full database
    rollbacks between test cases:

    async def test_homepage(client):
        url = app.url_path_for('homepage')
        response = await client.get(url)
        assert response.status_code == 200
    """
    async with TestClient(app=app) as client:
        yield client
#!/usr/bin/env python3

import asyncio
import typing
from types import TracebackType

from httpx import Client
from httpx.dispatch.asgi import ASGIDispatch
from starlette.types import ASGIApp


class TestClient(Client):
    __test__ = False
    token: str = None

    def __init__(self, app: ASGIApp, *args, **kwargs) -> None:
        self.app = app
        super().__init__(
            dispatch=ASGIDispatch(app=app),
            base_url='http://testserver',
            *args,
            **kwargs
        )


    async def ensure_authed(self):
        if self.token is None:  # TODO: expiry
            req = self.build_request(
                'POST',
                f'{self.base_url}/token',
                json={
                    "grant_type": "client_credentials",
                    "client_id": "dddaffc6-c786-48b7-8e3c-9f8472119939",
                    "client_secret": "password"
                }
            )
            resp = await self.send(req)
            assert resp.status_code == 200
            self.token = resp.json()['access_token']

        if 'Authorization' not in self.headers:
            self.headers = self.merge_headers({
                'Authorization': f'Bearer {self.token}'
            })


    async def request(self, *args, **kwargs):
        await self.ensure_authed()
        return await super().request(*args, **kwargs)


    async def lifespan(self) -> None:
        """ https://asgi.readthedocs.io/en/latest/specs/lifespan.html """
        scope = {'type': 'lifespan'}
        await self.app(scope, self.recv_queue.get, self.send_queue.put)
        await self.send_queue.put(None)


    async def wait_startup(self) -> None:
        await self.recv_queue.put({'type': 'lifespan.startup'})
        message = await self.send_queue.get()
        assert message['type'] in {
            'lifespan.startup.complete',
            'lifespan.startup.failed',
        }
        if message['type'] == 'lifespan.startup.failed':
            message = await self.send_queue.get()
            if message is None:
                self.task.result()


    async def wait_shutdown(self) -> None:
        await self.recv_queue.put({'type': 'lifespan.shutdown'})
        message = await self.send_queue.get()
        if message is None:
            self.task.result()

        assert message['type'] == 'lifespan.shutdown.complete'
        await self.task


    async def __aenter__(self) -> Client:
        self.send_queue = asyncio.Queue()
        self.recv_queue = asyncio.Queue()
        self.task: asyncio.Task = asyncio.create_task(self.lifespan())
        await self.wait_startup()
        return self


    async def __aexit__(
        self,
        exc_type: typing.Type[BaseException] = None,
        exc_value: BaseException = None,
        traceback: TracebackType = None,
    ) -> None:
        await self.wait_shutdown()
        await self.close()

@florimondmanca
Copy link
Member Author

Not sure if I mentioned it here already, but I pushed asgi-lifespan as a way to run lifespan event handlers, eg alongside an HTTPX AsyncClient.

@adriangb
Copy link
Member

It would be really nice to have an HTTPX based client that is framework independent and pulls in asgi-lifespan. That way each framework doesn't need to re-invent the wheel, and migrating between them becomes easier because tests are (more) decoupled from the framework.

It'd be extra nice if it could do what the current test client does and allow running async apps w/ a sync client but also support sync clients. Again, imagine migrating a flask app to starlette where there are 0 changes in your test except that your app is now an async starlette app instead of a flask one.

@euri10
Copy link
Member

euri10 commented Mar 20, 2022

I almost never used the Starlette built-in TestClient and always test things with httpx,

@kontsaki
Copy link

kontsaki commented Apr 5, 2022

Hello, @euri10 which is your approach to run the app concurrently?

@Kludex
Copy link
Member

Kludex commented Sep 6, 2022

This will be available in Starlette 0.21.0.

aguschin pushed a commit to iterative/mlem that referenced this issue Nov 17, 2022
from 0.21 starlette needs httpx for test client
encode/starlette#652
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testclient TestClient-related
Projects
None yet
Development

Successfully merging a pull request may close this issue.