Open cdunklau opened 4 years ago
So at least with tryFirst=True
, a user provided pytest_configure()
seems able to effect the change. Despite the exception being painfully generic (each place it is used should inherit from it to allow specificity!) I think it's still reasonable to use. We just need to either find a way to be specific about what we catch by inspecting it or be clear in the message that we are guessing as to the actual cause.
Ref https://github.com/pytest-dev/pytest-twisted/pull/75#discussion_r380495375 for the gory details, but I'll try to summarize, as there are a few similar yet distinct issues here. First...
The Background
Twisted's
AsyncioSelectorReactor
usesasyncio.get_event_loop
if no event loop is supplied to it explicitly. This is reasonable everywhere... except Windows, because it's the only platform with a non-"selector" event loop implementation in the stdlib:ProactorEventLoop
, which is incompatible withAsyncioSelectorReactor
. There does not appear to be a Twisted implementation of a proactor-friendlyasyncio
reactor (although Twisted has a normal IOCP-capable reactor).Before Python 3.8, the default event loop on windows is a
SelectorEventLoop
, so users would have to change the event loop policy to notice anything wrong.Python 3.8 changed that default to
ProactorEventLoop
, so now without any user input, Twisted'sAsyncioSelectorReactor
is broken by way ofNotImplementedError
from the (unimplemented)add_reader
method. To avoid this, users must either explicitly set the event loop policy back to one that gives selector loops, or create a loop distinct from the event loop policy (which I have a feeling isn't supported with the built-in policies) and pass it explicitly intoAsyncioSelectorReactor
Pytest-twisted needs to react (haha) to this in some fashion, in some way better than "belch NotImplementedError from the depths of twisted and asyncio", a la
The Battleground
One option is to unconditionally switch the policy:
This certainly does the job, but is rather heavy-handed. @cdunklau is not a huge fan, and suggested feature detection over version checking, and refusing the temptation to guess:
This is not pretty, as @altendky rightfully points out: "
NotImplementedError
isn't very specific".The Road to Reconciliation
Once #75 lands, we should have a working (if godawfully slow) CI rig for experimentation via PRs, and maybe get some speedup by switching to another provider... then we'll want to start off by adding some test cases that validate specifically for Windows that:
A user is able to reasonably change the event loop policy before pytest-twisted has a chance to make any decisions about it. This point is super important, as it will tell us what kinds of solutions actually bring value.
pytest_configure
hook, a user might not even be able to swap the policy before the fault is triggered.reactor_installers[config.getoption("reactor")]()
call would become necessary.The default event loop policy works with pytest-twisted in Py 3 < 3.8, and on 3.8+ either breaks in a friendlyish way or magically works (there's my lack of objectivity showing again 😬), depending on the solution chosen.
Explicitly setting the policy to
asyncio.WindowsProactorEventLoopPolicy
on all supported 3.x Pythons either breaks in a friendlyish way or is magically worked around.