Figure out how to handle asyncioreactor on windows: ProactorEventLoop, loop policies, py38 oh my!

[cdunklau]

Ref #75 (comment) for the gory details, but I'll try to summarize, as there are a few similar yet distinct issues here. First...

The Background

1. Twisted's AsyncioSelectorReactor uses asyncio.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 with AsyncioSelectorReactor. There does not appear to be a Twisted implementation of a proactor-friendly asyncio reactor (although Twisted has a normal IOCP-capable reactor).

2. 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.

3. Python 3.8 changed that default to ProactorEventLoop, so now without any user input, Twisted's AsyncioSelectorReactor is broken by way of NotImplementedError 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 into AsyncioSelectorReactor

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

<pytest_twisted.init_asyncio_reactor is called>

  File "c:\projects\pytest-twisted\.tox\py38-asyncioreactor\lib\site-packages\twisted\internet\asyncioreactor.py", line 320, in install
    reactor = AsyncioSelectorReactor(eventloop)
  File "c:\projects\pytest-twisted\.tox\py38-asyncioreactor\lib\site-packages\twisted\internet\asyncioreactor.py", line 69, in __init__
    super().__init__()
  File "c:\projects\pytest-twisted\.tox\py38-asyncioreactor\lib\site-packages\twisted\internet\base.py", line 571, in __init__
    self.installWaker()
  File "c:\projects\pytest-twisted\.tox\py38-asyncioreactor\lib\site-packages\twisted\internet\posixbase.py", line 286, in installWaker
    self.addReader(self.waker)
  File "c:\projects\pytest-twisted\.tox\py38-asyncioreactor\lib\site-packages\twisted\internet\asyncioreactor.py", line 151, in addReader
    self._asyncioEventloop.add_reader(fd, callWithLogger, reader,
  File "C:\Python38-x64\Lib\asyncio\events.py", line 501, in add_reader
    raise NotImplementedError
NotImplementedError

The Battleground

One option is to unconditionally switch the policy:

def init_asyncio_reactor():
    if sys.platform == 'win32':
        if sys.version_info >= (3, 8):
            # If twisted releases a fix/workaround we can check that version too
            # https://twistedmatrix.com/trac/ticket/9766
            import asyncio

            selector_policy = asyncio.WindowsSelectorEventLoopPolicy()
            asyncio.set_event_loop_policy(selector_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:

def init_asyncio_reactor():
    from twisted.internet import asyncioreactor
    try:
        _install_reactor(
            reactor_installer=asyncioreactor.install,
            reactor_type=asyncioreactor.AsyncioSelectorReactor,
        )
    except NotImplementedError as e:
        raise RuntimeError(
            "Failed to install AsyncioSelectorReactor. "
            "If you're on windows and python 3.8+, you might need to change "
            "the event loop policy to WindowsSelectorEventLoopPolicy."
        ) from e  # modulo `six.raise_from` for py2 support

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:

1. 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.

• I have a feeling that because the misbehaving function winds up getting called by the pytest_configure hook, a user might not even be able to swap the policy before the fault is triggered.
• My interpretation of that hook's reference is that it is call first for plugins (us) and then for the initial conftest file (the reasonable place for the user to make the tweak)... assuming that's correct, in order to allow the user even the chance, some kind of deferral (haha) mechanism for the reactor_installers[config.getoption("reactor")]() call would become necessary.

2. 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.

3. Explicitly setting the policy to asyncio.WindowsProactorEventLoopPolicy on all supported 3.x Pythons either breaks in a friendlyish way or is magically worked around.

[altendky]

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.