Keep the devmode logging bootstrap idempotent across re-execution

pcapkit/utilities/logging.py

@@ -336,14 +336,27 @@ def configure(level: 'Optional[Union[int, str]]' = None, *,
 # alone. Calling reset() here instead would detach a host application's
 # handlers merely because it imported pcapkit after configuring logging.
 #
-# The membership tests also make re-execution idempotent, which the test suite
-# relies on when it exercises import-time behaviour: neither the NullHandler
-# nor the devmode stderr handler can be stacked twice.
+# Re-execution has to be idempotent too, which the test suite relies on when it
+# exercises import-time behaviour. That rules out an identity test for the
+# devmode handler: ``handler`` is built when this module runs, so re-executing
+# it yields a *new* object every time and ``handler not in logger.handlers``
+# would be true on each pass, stacking one stderr handler per reload. The test
+# is therefore on what would actually duplicate -- a stream handler already
+# writing to the same stream.
 if not logger.handlers:
     logger.addHandler(logging.NullHandler())
 
+
+def _writes_to(candidate: 'logging.Handler', stream: 'Any') -> 'bool':
+    """Whether ``candidate`` is a stream handler already writing to ``stream``."""
+    return (
+        isinstance(candidate, logging.StreamHandler) and
+        getattr(candidate, 'stream', None) is stream
+    )
+
+
 if DEVMODE:
     # development mode keeps the historical behaviour: everything, on stderr
     logger.setLevel(logging.DEBUG)
-    if handler not in logger.handlers:
+    if not any(_writes_to(installed, handler.stream) for installed in logger.handlers):
         logger.addHandler(handler)

tests/utilities/test_logging.py

@@ -140,6 +140,42 @@ def test_re_executing_the_module_does_not_stack_handlers(self) -> None:
 
         self.assertEqual(len(logging.getLogger(ROOT).handlers), 1)
 
+    def test_re_executing_the_module_under_devmode_does_not_stack_handlers(self) -> None:
+        """The devmode bootstrap must be idempotent across re-execution.
+
+        ``handler`` is built when the module runs, so a *new* object exists on
+        every re-execution and an identity test against
+        ``logger.handlers`` would be true each time, adding one stderr handler
+        per reload. The guard therefore tests for a handler already writing to
+        the same stream.
+
+        """
+        root = logging.getLogger(ROOT)
+
+        # ``setUp``/``tearDown`` own this: the env var is saved and restored
+        # there, and ``pristine()`` resets the logger afterwards. Restoring it
+        # from an ``addCleanup`` instead would run *after* ``tearDown`` and so
+        # undo that reset, leaving the logger in this test's state rather than
+        # the neutral one the next test expects.
+        os.environ['PCAPKIT_DEVMODE'] = '1'
+
+        def streams() -> 'int':
+            return len([handler for handler in root.handlers
+                        if isinstance(handler, logging.StreamHandler)
+                        and not isinstance(handler, logging.NullHandler)])
+
+        module = load_module('pcapkit.utilities.logging', 'pcapkit/utilities/logging.py')
+        self.assertTrue(module.DEVMODE)
+        self.assertEqual(streams(), 1)
+
+        for _ in range(3):
+            load_module('pcapkit.utilities.logging', 'pcapkit/utilities/logging.py')
+
+        # one stderr handler however many times the module is executed, and the
+        # level devmode asks for is still in place
+        self.assertEqual(streams(), 1)
+        self.assertEqual(root.level, logging.DEBUG)
+
 
 class LoggerHierarchyTests(unittest.TestCase):
     """Per-module loggers, so a consumer can address one subtree at a time."""