starlette icon indicating copy to clipboard operation
starlette copied to clipboard

Add async test client

Open davidbrochart opened this issue 7 months ago • 7 comments

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 https://github.com/encode/starlette/discussions/2935.

Checklist

  • [x] I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • [x] 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.

davidbrochart avatar May 15 '25 08:05 davidbrochart

Nice! 😁

Kludex avatar May 15 '25 09:05 Kludex

Should all tests also be run with the AsyncTestClient?

davidbrochart avatar May 15 '25 10:05 davidbrochart

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

davidbrochart avatar May 15 '25 11:05 davidbrochart

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.

Kludex avatar May 15 '25 11:05 Kludex

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 avatar May 15 '25 21:05 davidbrochart

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 avatar May 16 '25 09:05 davidbrochart

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

davidbrochart avatar May 16 '25 10:05 davidbrochart