Skip to content

Conversation

@ykuzma1
Copy link

@ykuzma1 ykuzma1 commented Apr 20, 2019

This patch works by failing over to a ThreadPoolExecutor in cases where the expected event loop is already running. Notably this happens when calling request.getfixturevalue(argname) because it dynamically calls a fixture that needs to be setup on an event loop that's already running the asynchronous pytest tests.

…time error' by failing over to a ThreadPoolExecutor in cases where the expected event loop is already running. Notably this happens when calling 'request.getfixturevalue(argname)' because it dynamically calls a fixture that needs to be setup on an event loop that's already running pytest async tests
@ykuzma1 ykuzma1 force-pushed the 112-dynamic-fixture branch from 3ddb7f5 to e2dbfba Compare April 20, 2019 19:15
@ykuzma1
Copy link
Author

ykuzma1 commented Apr 20, 2019

Sorry, I was a bit premature. I was hoping to avoid changing the behavior of the existing code as much as possible by trying to run setup() as before and only falling back to running within the Executor if a RuntimeException occurred. Unfortunately, this was running setup() multiple times (🤦‍♂️) causing issues with test setup and tear-down. This means that it has to run setup() in the Executor no matter what.

The only other option is detecting whether the fixture was called dynamically from within pytest_fixture_setup(), but I'm not aware of any way to do that.

@simonfagerholm
Copy link
Contributor

@ykuzma1 How about adding a marker that can be identified from within the hooks?
So if you know that your testcase needs to call fixture dynamically you can mark that testcase

@asvetlov
Copy link
Contributor

asvetlov commented Jan 6, 2022

I guess the PR should be closed.
Running an async fixture setup phase in a thread pool violates the asyncio usage rules.
Thanks for the PR anyway!

@Tinche Tinche closed this Jan 6, 2022
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