-
-
Notifications
You must be signed in to change notification settings - Fork 949
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
TestClient accepts backend and backend_options as arguments #1210
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix, I think it's worth removing the classvar and using a functools.partial object for the tests
@@ -381,13 +381,11 @@ def receive_json(self, mode: str = "text") -> typing.Any: | |||
|
|||
class TestClient(requests.Session): | |||
__test__ = False # For pytest to not discover this up. | |||
|
|||
#: These options are passed to `anyio.start_blocking_portal()` | |||
#: These are the default options for the constructor arguments | |||
async_backend: typing.Dict[str, typing.Any] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this class-level declaration still? Class vars always confuse me, but this means somebody can do:
TestClient.async_backend["backend"] = "trio"
to default all future TestClient
instances to use the Trio backend, right?
I know removing this would be slightly changing the API, however in the code examples we gave we only modified instance attributes and that usage should stay the same. What's your thinking around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed it in my followup PR: #1211
merged in #1211 |
As implemented in #1157,
TestClient.async_backend
was a class-level attribute. This made it easy to monkeypatch for use in starlette tests, but makes it difficult/impossible to run different test client backends concurrently (in threads).This PR changes TestClient to accept a
backend
andbackend_options
arguments which allow a user to specify the backend options for only that TestClient instance. The class attribute will remain as the default for these arguments.Fixes #1209