-
Notifications
You must be signed in to change notification settings - Fork 5
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
Replace mark.async_timeout with fixtures #20
Conversation
* Change exported api * Enabling requires explicitly adding `alt_pytest_asyncio.enable` to pytest plugins list if the plugin isn't being manually enabled
e4a6a69
to
0399979
Compare
Fix #19 The pytest library is deprecating the ability to set marks on fixtures, which breaks how alt-pytest-asyncio currently determines async timeouts. This change rewrites how all of that works to simplify and to make it so that fixtures and tests modify that timeout by requesting an `async_timeout` fixture which provides a `set_timeout_seconds` method for setting the timeout for that fixture/test. Also this provides the ability to set defaults via `default_async_timeout` fixtures. With a slight complication that the default is looked up in order via * default_async_timeout * class_default_async_timeout * module_default_async_timeout * package_default_async_timeout * session_default_async_timeout With the start point depending on the scope of the fixture. So a test will always start from the beginning of the order, but a "scope=module" fixture would start from `module_default_async_timeout` because a module scope fixture can't access class or function level fixtures, etc.
@delfick Just because we can't mark fixtures doesn't mean we can't continue marking tests. Any reason you didn't leave the mark API for test functions and add the new fixture API for setting the timeout for a fixture? It seems like an unnecessarily breaking change, and it's a less elegant API (in my opinion). |
@brianmedigate How much are you changing tests to have their own unique timeout? |
Not a ton, but we have a few tests which are slower than the default, so they currently have marks with longer timeouts, and I find it cleaner and easier to understand at a glance when it's done declaratively with a mark on the test rather than programatically inside the test function. |
@brianmedigate yeah, I got rid of the marks altogether cause from my perspective the consistency is more important and less confusing and it should be pretty special case to change it per test, and for a group of tests it's possible to create a
|
ok, thank you! |
Fix #19
The pytest library is deprecating the ability to set marks on fixtures, which breaks how alt-pytest-asyncio currently determines async timeouts.
This change rewrites how all of that works to simplify and to make it so that fixtures and tests modify that timeout by requesting an
async_timeout
fixture which provides aset_timeout_seconds
method for setting the timeout for that fixture/test.Also this provides the ability to set defaults via
default_async_timeout
fixtures.With a slight complication that the default is looked up in order via
With the start point depending on the scope of the fixture. So a test will always start from the beginning of the order, but a "scope=module" fixture would start from
module_default_async_timeout
because a module scope fixture can't access class or function level fixtures, etc.At the same time, I'm making it so that enabling the plugin requires adding
alt_pytest_asyncio.enable
to the pytest plugin list (this is a different tradeoff to one I was already making to cater for the ability to create the AltPytestAsyncioPlugin with a managed loop)And I've changed the exported api of the plugin a little as mentioned in the README.