Skip to content

TestClient accepts backend and backend_options as arguments to constructor#1211

Merged
graingert merged 8 commits intoKludex:masterfrom
graingert:testclient-async_backend
Jun 28, 2021
Merged

TestClient accepts backend and backend_options as arguments to constructor#1211
graingert merged 8 commits intoKludex:masterfrom
graingert:testclient-async_backend

Conversation

@graingert
Copy link
Contributor

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
@graingert graingert force-pushed the testclient-async_backend branch from 480fb3e to a07b468 Compare June 23, 2021 20:27
@graingert graingert force-pushed the testclient-async_backend branch from a07b468 to 070fba9 Compare June 23, 2021 20:30
@graingert graingert force-pushed the testclient-async_backend branch from 070fba9 to 3498c9e Compare June 23, 2021 20:34
@graingert graingert changed the title avoid monkeypatching TestClient in tests remove monkeypatching TestClient interface Jun 23, 2021
@uSpike
Copy link
Contributor

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
Contributor 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
Contributor

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
@graingert graingert requested a review from JayH5 June 28, 2021 20:18
Copy link
Contributor

@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"?

Co-authored-by: Jamie Hewland <jhewland@gmail.com>
@graingert
Copy link
Contributor 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
Contributor

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 Kludex: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.

4 participants