Copilot review

Author: aclark4life

test/asynchronous/test_periodic_executor.py

@@ -30,30 +30,27 @@
 _IS_SYNC = False
 
 
-def _make_executor(interval=30.0, min_interval=0.01, target=None, name="test"):
-    if target is None:
-
-        async def target():
-            return True
-
-    return AsyncPeriodicExecutor(
-        interval=interval, min_interval=min_interval, target=target, name=name
-    )
+class TestAsyncPeriodicExecutor(AsyncUnitTest):
+    def _make_executor(self, interval=30.0, min_interval=0.01, target=None, name="test"):
+        if target is None:
 
+            async def target():
+                return True
 
-class TestAsyncPeriodicExecutor(AsyncUnitTest):
-    async def asyncSetUp(self):
-        self.executor = None
+        executor = AsyncPeriodicExecutor(
+            interval=interval, min_interval=min_interval, target=target, name=name
+        )
+        self.addAsyncCleanup(self._close_executor, executor)
+        return executor
 
-    async def asyncTearDown(self):
-        if self.executor is not None:
-            self.executor.close()
-            await self.executor.join(timeout=2)
+    async def _close_executor(self, executor):
+        executor.close()
+        await executor.join(timeout=2)
 
     async def test_join_without_open_is_safe(self):
-        self.executor = _make_executor()
+        executor = self._make_executor()
         try:
-            await self.executor.join(timeout=0.01)
+            await executor.join(timeout=0.01)
         except Exception as e:
             self.fail(f"join() raised unexpected Exception {e}")
 
@@ -67,9 +64,9 @@ async def target():
             ran.set()
             return False
 
-        self.executor = _make_executor(target=target)
-        self.executor.open()
-        await self.executor.join(timeout=2)
+        executor = self._make_executor(target=target)
+        executor.open()
+        await executor.join(timeout=2)
         self.assertTrue(ran.is_set(), "target never ran")
 
     async def test_target_exception_stops_executor(self):
@@ -80,15 +77,15 @@ async def target():
             call_count += 1
             raise RuntimeError("error")
 
-        self.executor = _make_executor(target=target)
+        executor = self._make_executor(target=target)
         # Suppress threading.excepthook so the intentional RuntimeError raised
         # in the worker thread is not surfaced by pytest's threadexception
         # plugin (no-op for the async executor since no thread is involved).
         orig_excepthook = threading.excepthook
         threading.excepthook = lambda _args: None
         try:
-            self.executor.open()
-            await self.executor.join(timeout=2)
+            executor.open()
+            await executor.join(timeout=2)
         finally:
             threading.excepthook = orig_excepthook
         self.assertEqual(call_count, 1, "target should stop after exception")
@@ -102,10 +99,10 @@ async def target():
                 return False
             return True
 
-        self.executor = _make_executor(interval=30.0, min_interval=0.001, target=target)
-        self.executor.skip_sleep()
-        self.executor.open()
-        await self.executor.join(timeout=3)
+        executor = self._make_executor(interval=30.0, min_interval=0.001, target=target)
+        executor.skip_sleep()
+        executor.open()
+        await executor.join(timeout=3)
         self.assertGreaterEqual(len(call_times), 2)
         self.assertLess(call_times[1] - call_times[0], 5.0)
 
@@ -123,15 +120,15 @@ async def target():
                 woken.set()
             return call_count < 2
 
-        self.executor = _make_executor(interval=30.0, min_interval=0.01, target=target)
-        self.executor.open()
+        executor = self._make_executor(interval=30.0, min_interval=0.01, target=target)
+        executor.open()
         if _IS_SYNC:
             woken.wait(timeout=2)
         else:
             assert isinstance(woken, asyncio.Event)
             await asyncio.wait_for(woken.wait(), timeout=2)
-        self.executor.wake()
-        await self.executor.join(timeout=3)
+        executor.wake()
+        await executor.join(timeout=3)
         self.assertGreaterEqual(call_count, 2)
 
     async def test_open_after_target_returns_false(self):
@@ -142,11 +139,11 @@ async def target():
             called += 1
             return False
 
-        self.executor = _make_executor(target=target)
-        self.executor.open()
-        await self.executor.join(timeout=2)
-        self.executor.open()
-        await self.executor.join(timeout=2)
+        executor = self._make_executor(target=target)
+        executor.open()
+        await executor.join(timeout=2)
+        executor.open()
+        await executor.join(timeout=2)
         self.assertGreaterEqual(called, 2)
 
 

test/test_periodic_executor.py

@@ -30,28 +30,27 @@
 _IS_SYNC = True
 
 
-def _make_executor(interval=30.0, min_interval=0.01, target=None, name="test"):
-    if target is None:
-
-        def target():
-            return True
-
-    return PeriodicExecutor(interval=interval, min_interval=min_interval, target=target, name=name)
+class TestPeriodicExecutor(UnitTest):
+    def _make_executor(self, interval=30.0, min_interval=0.01, target=None, name="test"):
+        if target is None:
 
+            def target():
+                return True
 
-class TestPeriodicExecutor(UnitTest):
-    def setUp(self):
-        self.executor = None
+        executor = PeriodicExecutor(
+            interval=interval, min_interval=min_interval, target=target, name=name
+        )
+        self.addCleanup(self._close_executor, executor)
+        return executor
 
-    def tearDown(self):
-        if self.executor is not None:
-            self.executor.close()
-            self.executor.join(timeout=2)
+    def _close_executor(self, executor):
+        executor.close()
+        executor.join(timeout=2)
 
     def test_join_without_open_is_safe(self):
-        self.executor = _make_executor()
+        executor = self._make_executor()
         try:
-            self.executor.join(timeout=0.01)
+            executor.join(timeout=0.01)
         except Exception as e:
             self.fail(f"join() raised unexpected Exception {e}")
 
@@ -65,9 +64,9 @@ def target():
             ran.set()
             return False
 
-        self.executor = _make_executor(target=target)
-        self.executor.open()
-        self.executor.join(timeout=2)
+        executor = self._make_executor(target=target)
+        executor.open()
+        executor.join(timeout=2)
         self.assertTrue(ran.is_set(), "target never ran")
 
     def test_target_exception_stops_executor(self):
@@ -78,15 +77,15 @@ def target():
             call_count += 1
             raise RuntimeError("error")
 
-        self.executor = _make_executor(target=target)
+        executor = self._make_executor(target=target)
         # Suppress threading.excepthook so the intentional RuntimeError raised
         # in the worker thread is not surfaced by pytest's threadexception
         # plugin (no-op for the async executor since no thread is involved).
         orig_excepthook = threading.excepthook
         threading.excepthook = lambda _args: None
         try:
-            self.executor.open()
-            self.executor.join(timeout=2)
+            executor.open()
+            executor.join(timeout=2)
         finally:
             threading.excepthook = orig_excepthook
         self.assertEqual(call_count, 1, "target should stop after exception")
@@ -100,10 +99,10 @@ def target():
                 return False
             return True
 
-        self.executor = _make_executor(interval=30.0, min_interval=0.001, target=target)
-        self.executor.skip_sleep()
-        self.executor.open()
-        self.executor.join(timeout=3)
+        executor = self._make_executor(interval=30.0, min_interval=0.001, target=target)
+        executor.skip_sleep()
+        executor.open()
+        executor.join(timeout=3)
         self.assertGreaterEqual(len(call_times), 2)
         self.assertLess(call_times[1] - call_times[0], 5.0)
 
@@ -121,15 +120,15 @@ def target():
                 woken.set()
             return call_count < 2
 
-        self.executor = _make_executor(interval=30.0, min_interval=0.01, target=target)
-        self.executor.open()
+        executor = self._make_executor(interval=30.0, min_interval=0.01, target=target)
+        executor.open()
         if _IS_SYNC:
             woken.wait(timeout=2)
         else:
             assert isinstance(woken, asyncio.Event)
             asyncio.wait_for(woken.wait(), timeout=2)
-        self.executor.wake()
-        self.executor.join(timeout=3)
+        executor.wake()
+        executor.join(timeout=3)
         self.assertGreaterEqual(call_count, 2)
 
     def test_open_after_target_returns_false(self):
@@ -140,11 +139,11 @@ def target():
             called += 1
             return False
 
-        self.executor = _make_executor(target=target)
-        self.executor.open()
-        self.executor.join(timeout=2)
-        self.executor.open()
-        self.executor.join(timeout=2)
+        executor = self._make_executor(target=target)
+        executor.open()
+        executor.join(timeout=2)
+        executor.open()
+        executor.join(timeout=2)
         self.assertGreaterEqual(called, 2)