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

TestClient accepts backend and backend_options as arguments to constructor #1211

Merged
merged 8 commits into from Jun 28, 2021

Conversation

graingert
Copy link
Member

No description provided.

@graingert graingert changed the title TestClient accepts backend and backend_options as arguments to constructor avoid monkeypatching TestClient in tests Jun 23, 2021
@graingert graingert requested a review from uSpike June 23, 2021 20:03
setup.py Show resolved Hide resolved
@graingert graingert changed the title avoid monkeypatching TestClient in tests remove monkeypatching TestClient interface Jun 23, 2021
@uSpike
Copy link
Member

uSpike commented Jun 23, 2021

It seems you've removed the parametrization of anyio_backend that ran the tests on both trio and asyncio.

@graingert
Copy link
Member Author

graingert commented Jun 23, 2021

It seems you've removed the parametrization of anyio_backend that ran the tests on both trio and asyncio.

anyio_backend is set to ["trio", "asyncio"] by default:https://github.com/agronholm/anyio/blob/c90169c0bbf0adc53234bde218e265f1b9157797/src/anyio/pytest_plugin.py#L140-L142

@uSpike
Copy link
Member

uSpike commented Jun 23, 2021

Hm you're right, the default for anyio_backend is to use all backends: https://github.com/agronholm/anyio/blob/3.2.0/src/anyio/pytest_plugin.py#L140

By deleting it you're effectively testing on all backends supported by anyio (but also not explicitly passing options).

@graingert graingert requested a review from JayH5 June 28, 2021 13:42
starlette/testclient.py Show resolved Hide resolved
tests/conftest.py Show resolved Hide resolved
@graingert graingert requested a review from JayH5 June 28, 2021 20:18
Copy link
Member

@JayH5 JayH5 left a comment

Choose a reason for hiding this comment

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

Thanks. Only thing is the PR title--does this count as "monkeypatching"?

starlette/testclient.py Outdated Show resolved Hide resolved
Co-authored-by: Jamie Hewland <jhewland@gmail.com>
@graingert
Copy link
Member Author

Thanks. Only thing is the PR title--does this count as "monkeypatching"?

isn't that what this is?

TestClient.async_backend["backend"] = "trio"

@JayH5
Copy link
Member

JayH5 commented Jun 28, 2021

isn't that what this is?

🤔 I guess technically it is monkey patching since it's modifying a class attribute at runtime. Always thought of monkey patching as a "bigger hammer" where you modify a method at runtime and potentially do so in a way that wasn't intended by the original author.

@graingert graingert changed the title remove monkeypatching TestClient interface TestClient accepts backend and backend_options as arguments to constructor Jun 28, 2021
@graingert graingert merged commit d222b87 into encode:master Jun 28, 2021
@graingert graingert deleted the testclient-async_backend branch June 28, 2021 20:36
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.

None yet

4 participants