From 08effe8682d62c9050f304491cc0faa62766bf97 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 14:46:06 -0400 Subject: [PATCH 1/5] news fragment --- src/twisted/newsfragments/11694.misc | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 src/twisted/newsfragments/11694.misc diff --git a/src/twisted/newsfragments/11694.misc b/src/twisted/newsfragments/11694.misc new file mode 100644 index 00000000000..e69de29bb2d From 46da05652f0ed062cc69bfdea9ebb515941c0b85 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 14:46:19 -0400 Subject: [PATCH 2/5] incidental formatting fix --- src/twisted/internet/test/test_time.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/twisted/internet/test/test_time.py b/src/twisted/internet/test/test_time.py index d68176cd79f..0e847f6e34c 100644 --- a/src/twisted/internet/test/test_time.py +++ b/src/twisted/internet/test/test_time.py @@ -43,7 +43,7 @@ def eventSource(reactor, event): else: raise SkipTest( - "Do not know how to synthesize non-time event to " "stop the test" + "Do not know how to synthesize non-time event to stop the test" ) # Pick a pretty big delay. From 5ff4201b7fe9af692f81c5f3bd5c886f5b8f0394 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 14:47:24 -0400 Subject: [PATCH 3/5] Move the work-around into reactormixins.py Now AsyncioSelectReactor tests don't share a loop with each other so they're less likely to leak state onto each other. --- src/twisted/internet/test/reactormixins.py | 29 ++++++++++++++++++++-- src/twisted/internet/test/test_threads.py | 16 ++---------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/twisted/internet/test/reactormixins.py b/src/twisted/internet/test/reactormixins.py index b5984669588..4cec6618ae5 100644 --- a/src/twisted/internet/test/reactormixins.py +++ b/src/twisted/internet/test/reactormixins.py @@ -18,7 +18,7 @@ import os import signal import time -from typing import Dict, Optional, Sequence, Type, Union +from typing import TYPE_CHECKING, Dict, Optional, Sequence, Type, Union from zope.interface import Interface @@ -30,6 +30,11 @@ from twisted.trial.unittest import SkipTest, SynchronousTestCase from twisted.trial.util import DEFAULT_TIMEOUT_DURATION, acquireAttribute +if TYPE_CHECKING: + # Only bring in this name to support the type annotation below. We don't + # really want to import a reactor module this early at runtime. + from twisted.internet.asyncioreactor import AsyncioSelectorReactor + # Access private APIs. try: from twisted.internet import process as _process @@ -153,7 +158,7 @@ class ReactorBuilder: ] ) - _reactors.append("twisted.internet.asyncioreactor.AsyncioSelectorReactor") + _reactors.append("twisted.internet.test.reactormixins.asyncioreactor") if platform.isMacOSX(): _reactors.append("twisted.internet.cfreactor.CFReactor") @@ -366,3 +371,23 @@ class testcase(cls, SynchronousTestCase): # type: ignore[valid-type,misc] testcase.__qualname__ = ".".join(cls.__qualname__.split()[0:-1] + [name]) classes[testcase.__name__] = testcase return classes + + +def asyncioreactor() -> "AsyncioSelectorReactor": + """ + Make a new asyncio reactor associated with a new event loop. + + The test suite prefers this constructor because having a new event loop + for each reactor provides better test isolation. The real constructor + prefers to re-use (or create) a global loop because of how this interacts + with other asyncio-based libraries and applications (though maybe it + shouldn't). + """ + from asyncio import new_event_loop, set_event_loop + + from twisted.internet.asyncioreactor import AsyncioSelectorReactor + + loop = new_event_loop() + set_event_loop(loop) + + return AsyncioSelectorReactor(loop) diff --git a/src/twisted/internet/test/test_threads.py b/src/twisted/internet/test/test_threads.py index 68487ef0724..3b45a4cff86 100644 --- a/src/twisted/internet/test/test_threads.py +++ b/src/twisted/internet/test/test_threads.py @@ -172,20 +172,8 @@ def test_cleanUpThreadPoolEvenBeforeReactorIsRun(self): reactor = self.buildReactor() threadPoolRef = ref(reactor.getThreadPool()) reactor.fireSystemEvent("shutdown") - - if reactor.__class__.__name__ == "AsyncioSelectorReactor": - self.assertIsNone(reactor.threadpool) - # ReactorBase.__init__ sets self.crash as a 'shutdown' - # event, which in turn calls stop on the underlying - # asyncio event loop, which in turn sets a _stopping - # attribute on it that's only unset after an iteration of - # the loop. Subsequent tests can only reuse the asyncio - # loop if it's allowed to run and unset that _stopping - # attribute. - self.runReactor(reactor) - else: - gc.collect() - self.assertIsNone(threadPoolRef()) + gc.collect() + self.assertIsNone(threadPoolRef()) def test_isInIOThread(self): """ From a017e454c09af4221192eadfae5f2ab9703b3a53 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 15:46:08 -0400 Subject: [PATCH 4/5] Fix the asyncio reactor test builder 1. Give it a name that preserves the existing test names 2. Make it a static method so it doesn't raise TypeError when called by the builder. --- src/twisted/internet/test/reactormixins.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/twisted/internet/test/reactormixins.py b/src/twisted/internet/test/reactormixins.py index 4cec6618ae5..785bdf4c1a8 100644 --- a/src/twisted/internet/test/reactormixins.py +++ b/src/twisted/internet/test/reactormixins.py @@ -158,7 +158,7 @@ class ReactorBuilder: ] ) - _reactors.append("twisted.internet.test.reactormixins.asyncioreactor") + _reactors.append("twisted.internet.test.reactormixins.AsyncioSelectReactor") if platform.isMacOSX(): _reactors.append("twisted.internet.cfreactor.CFReactor") @@ -373,7 +373,8 @@ class testcase(cls, SynchronousTestCase): # type: ignore[valid-type,misc] return classes -def asyncioreactor() -> "AsyncioSelectorReactor": +@staticmethod +def AsyncioSelectReactor() -> "AsyncioSelectorReactor": """ Make a new asyncio reactor associated with a new event loop. @@ -382,6 +383,10 @@ def asyncioreactor() -> "AsyncioSelectorReactor": prefers to re-use (or create) a global loop because of how this interacts with other asyncio-based libraries and applications (though maybe it shouldn't). + + @note: This function's name deviates from the coding standard because the + name of the generated test cases is derived from it. The given name + reflects the name of the reactor the generated tests will cover. """ from asyncio import new_event_loop, set_event_loop From 00d2b7ae1ba375472da800d8bfe88e1376da41a8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 16:01:27 -0400 Subject: [PATCH 5/5] Shuffle the pieces around to avoid the staticmethod mypy error --- src/twisted/internet/test/reactormixins.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/twisted/internet/test/reactormixins.py b/src/twisted/internet/test/reactormixins.py index 785bdf4c1a8..5bfb781e7c9 100644 --- a/src/twisted/internet/test/reactormixins.py +++ b/src/twisted/internet/test/reactormixins.py @@ -33,7 +33,7 @@ if TYPE_CHECKING: # Only bring in this name to support the type annotation below. We don't # really want to import a reactor module this early at runtime. - from twisted.internet.asyncioreactor import AsyncioSelectorReactor + from twisted.internet import asyncioreactor # Access private APIs. try: @@ -158,7 +158,7 @@ class ReactorBuilder: ] ) - _reactors.append("twisted.internet.test.reactormixins.AsyncioSelectReactor") + _reactors.append("twisted.internet.test.reactormixins.AsyncioSelectorReactor") if platform.isMacOSX(): _reactors.append("twisted.internet.cfreactor.CFReactor") @@ -373,8 +373,7 @@ class testcase(cls, SynchronousTestCase): # type: ignore[valid-type,misc] return classes -@staticmethod -def AsyncioSelectReactor() -> "AsyncioSelectorReactor": +def asyncioSelectorReactor(self: object) -> "asyncioreactor.AsyncioSelectorReactor": """ Make a new asyncio reactor associated with a new event loop. @@ -384,15 +383,19 @@ def AsyncioSelectReactor() -> "AsyncioSelectorReactor": with other asyncio-based libraries and applications (though maybe it shouldn't). - @note: This function's name deviates from the coding standard because the - name of the generated test cases is derived from it. The given name - reflects the name of the reactor the generated tests will cover. + @param self: The L{ReactorBuilder} subclass this is being called on. We + don't use this parameter but we get called with it anyway. """ from asyncio import new_event_loop, set_event_loop - from twisted.internet.asyncioreactor import AsyncioSelectorReactor + from twisted.internet import asyncioreactor loop = new_event_loop() set_event_loop(loop) - return AsyncioSelectorReactor(loop) + return asyncioreactor.AsyncioSelectorReactor(loop) + + +# Give it an alias that makes the names of the generated test classes fit the +# pattern. +AsyncioSelectorReactor = asyncioSelectorReactor