Copilot + Noah review

Author: aclark4life

test/asynchronous/test_periodic_executor.py

@@ -30,38 +30,27 @@
 _IS_SYNC = False
 
 
-def _make_executor(interval=30.0, min_interval=0.01, target=None, name="test"):
-    if target is None:
+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
-
-    return AsyncPeriodicExecutor(
-        interval=interval, min_interval=min_interval, target=target, name=name
-    )
-
-
-class AsyncPeriodicExecutorTestBase(AsyncUnitTest):
-    async def asyncSetUp(self):
-        self.executor = None
-
-    async def asyncTearDown(self):
-        if self.executor is not None:
-            self.executor.close()
-            await self.executor.join(timeout=2)
+            async def target():
+                return True
 
+        executor = AsyncPeriodicExecutor(
+            interval=interval, min_interval=min_interval, target=target, name=name
+        )
+        self.addAsyncCleanup(self._close_executor, executor)
+        return executor
 
-class TestAsyncPeriodicExecutor(AsyncPeriodicExecutorTestBase):
-    async def test_repr_contains_class_and_name(self):
-        executor = _make_executor(name="exec")
-        executor_repr = repr(executor)
-        self.assertIn("AsyncPeriodicExecutor", executor_repr)
-        self.assertIn("exec", executor_repr)
+    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}")
 
@@ -75,47 +64,31 @@ 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):
-        if _IS_SYNC:
-            ran = threading.Event()
-            captured_exc: list = []
-            orig_excepthook = threading.excepthook
-
-            def _capture_excepthook(args):
-                captured_exc.append(args.exc_value)
-
-            threading.excepthook = _capture_excepthook
-            try:
-
-                def target():
-                    ran.set()
-                    raise RuntimeError("error")
-
-                self.executor = _make_executor(target=target)
-                self.executor.open()
-                self.executor.join(timeout=2)
-                self.assertTrue(ran.is_set(), "target never ran")
-            finally:
-                threading.excepthook = orig_excepthook
-            self.assertEqual(len(captured_exc), 1)
-            self.assertIsInstance(captured_exc[0], RuntimeError)
-        else:
-            call_count = 0
-
-            async def target():
-                nonlocal call_count
-                call_count += 1
-                raise RuntimeError("error")
+        call_count = 0
 
-            self.executor = _make_executor(target=target)
-            self.executor.open()
-            await self.executor.join(timeout=2)
-            self.assertEqual(call_count, 1, "target should stop after exception")
+        async def target():
+            nonlocal call_count
+            call_count += 1
+            raise RuntimeError("error")
+
+        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:
+            executor.open()
+            await executor.join(timeout=2)
+        finally:
+            threading.excepthook = orig_excepthook
+        self.assertEqual(call_count, 1, "target should stop after exception")
 
     async def test_skip_sleep_flag_skips_interval(self):
         call_times = []
@@ -126,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)
 
@@ -147,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):
@@ -166,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,36 +30,27 @@
 _IS_SYNC = True
 
 
-def _make_executor(interval=30.0, min_interval=0.01, target=None, name="test"):
-    if target is None:
+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
-
-    return PeriodicExecutor(interval=interval, min_interval=min_interval, target=target, name=name)
-
-
-class PeriodicExecutorTestBase(UnitTest):
-    def setUp(self):
-        self.executor = None
-
-    def tearDown(self):
-        if self.executor is not None:
-            self.executor.close()
-            self.executor.join(timeout=2)
+            def target():
+                return True
 
+        executor = PeriodicExecutor(
+            interval=interval, min_interval=min_interval, target=target, name=name
+        )
+        self.addCleanup(self._close_executor, executor)
+        return executor
 
-class TestPeriodicExecutor(PeriodicExecutorTestBase):
-    def test_repr_contains_class_and_name(self):
-        executor = _make_executor(name="exec")
-        executor_repr = repr(executor)
-        self.assertIn("PeriodicExecutor", executor_repr)
-        self.assertIn("exec", executor_repr)
+    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}")
 
@@ -73,47 +64,31 @@ 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):
-        if _IS_SYNC:
-            ran = threading.Event()
-            captured_exc: list = []
-            orig_excepthook = threading.excepthook
-
-            def _capture_excepthook(args):
-                captured_exc.append(args.exc_value)
-
-            threading.excepthook = _capture_excepthook
-            try:
-
-                def target():
-                    ran.set()
-                    raise RuntimeError("error")
-
-                self.executor = _make_executor(target=target)
-                self.executor.open()
-                self.executor.join(timeout=2)
-                self.assertTrue(ran.is_set(), "target never ran")
-            finally:
-                threading.excepthook = orig_excepthook
-            self.assertEqual(len(captured_exc), 1)
-            self.assertIsInstance(captured_exc[0], RuntimeError)
-        else:
-            call_count = 0
-
-            def target():
-                nonlocal call_count
-                call_count += 1
-                raise RuntimeError("error")
+        call_count = 0
 
-            self.executor = _make_executor(target=target)
-            self.executor.open()
-            self.executor.join(timeout=2)
-            self.assertEqual(call_count, 1, "target should stop after exception")
+        def target():
+            nonlocal call_count
+            call_count += 1
+            raise RuntimeError("error")
+
+        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:
+            executor.open()
+            executor.join(timeout=2)
+        finally:
+            threading.excepthook = orig_excepthook
+        self.assertEqual(call_count, 1, "target should stop after exception")
 
     def test_skip_sleep_flag_skips_interval(self):
         call_times = []
@@ -124,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)
 
@@ -145,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):
@@ -164,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)