diff --git a/pcapkit/utilities/logging.py b/pcapkit/utilities/logging.py index 15db274b50..f3afc403ff 100644 --- a/pcapkit/utilities/logging.py +++ b/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) diff --git a/tests/utilities/test_logging.py b/tests/utilities/test_logging.py index 4f0c747de4..2eaa476a95 100644 --- a/tests/utilities/test_logging.py +++ b/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."""