Skip to content

Add async test client #2937

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

davidbrochart
Copy link

Summary

This PR adds AsyncTestClient, an async equivalent of TestClient. The TestClient runs in a background thread in order to let the application run in the main thread, while the AsyncTestClient runs in a background task in the same thread as the application.
Currently, AsyncTestClient is almost a duplication of TestClient, with the replacement of the BlockingPortal with a TaskGroup. I'm not sure if/how some code could be shared.
See #2935.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@Kludex
Copy link
Member

Kludex commented May 15, 2025

Nice! 😁

@davidbrochart
Copy link
Author

Should all tests also be run with the AsyncTestClient?

@davidbrochart
Copy link
Author

Any idea about the best way to factorize code between TestClient and AsyncTestClient?

@Kludex
Copy link
Member

Kludex commented May 15, 2025

Should all tests also be run with the AsyncTestClient?

No, that would be too huge of a PR (but also, I don't think we should do it). You just need to test the client itself.

@davidbrochart
Copy link
Author

davidbrochart commented May 15, 2025

No, that would be too huge of a PR (but also, I don't think we should do it).

If we can parameterize tests so that they use both TestClient and AsyncTestClient, there won't be too many changes, just the extra await for AsyncTestClient.

You just need to test the client itself.

The thing is that we don't get enough code coverage for AsyncTestClient, with only test_asynctestclient.py:

Name                      Stmts   Miss  Cover   Missing
-------------------------------------------------------
starlette/testclient.py     574     54    91%   757, 790, 792-801, 807, 810, 813-817, 826-828, 841, 871-872, 895, 939-962, 982-984, 989-991, 995-996, 1006-1007, 1125, 1148, 1175, 1206, 1237, 1264, 1287, 1362, 1372
-------------------------------------------------------
TOTAL                      9911     54    99%

I checked that it was the same for TestClient, where these lines were not covered in test_testclient.py only:

Name                      Stmts   Miss  Cover   Missing
-------------------------------------------------------
starlette/testclient.py     576    284    51%   49, 73, 141, 143-152, 158, 161, 164-168, 177-179, 192, 224-225, 248, 292-315, 335-337, 343-345, 349-350, 360-361, 493, 516, 543, 574, 605, 632, 655, 726, 736...

The other tests bring full code coverage.

Factorizing code between TestClient and AsyncTestClient will raise code coverage and has to be done anyway, but I don't think it will be enough. Maybe we can port some tests to async to get full code coverage.

@davidbrochart
Copy link
Author

davidbrochart commented May 16, 2025

I was thinking about mixin classes that the test client and the async test client would use, but I'm not sure it's worth it. It would make the code more DRY, but not that much and at the cost of more complexity. Maybe the async test client should live in a separate asynctestclient.py file, and every future change to testclient.py should be ported to asynctestclient.py?
Alternatively, if the async test client is not used in starlette's tests then it should live in a different package? I could port starlette's tests there and get full code coverage. Maybe also give it a "better" name than AsyncTestClient, since it's really an HTTP-over-ASGI client.
What do you think?

@davidbrochart
Copy link
Author

Hmm it looks like https://github.com/vinissimus/async-asgi-testclient is just what I needed.


@pytest.fixture
def async_test_client_factory() -> AsyncTestClientFactory:
return functools.partial(
Copy link
Member

Choose a reason for hiding this comment

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

This partial is redundant (and probably the whole fixture too)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants