Skip to content

Commit e502c27

Browse files
committed
logging: capture each record once when propagate changes during a test
Records from a logger which was non-propagating at capture start were handled twice once the test enables Logger.propagate: pytest's capture handler ran directly on that logger and again via the root logger. Instead of attaching the real capture handler to initially non-propagating loggers, attach a lightweight proxy bound to each such logger and its ancestors which forwards to the real handler only while that logger currently has propagate disabled. The proxy reads the live propagate value per record, so a False -> True transition stops the direct handling as soon as the record can reach root, and a True -> False transition on those loggers is now captured as well. Fixes #15064
1 parent 6a9ba0f commit e502c27

4 files changed

Lines changed: 158 additions & 6 deletions

File tree

‎changelog/15064.bugfix.rst‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Fixed duplicate log records when a logger which was non-propagating at capture
2+
start enables ``logging.Logger.propagate`` during the test: the record was
3+
handled both by pytest's handler attached directly to that logger and by the
4+
one attached to the root logger. Capture (``caplog``, failure-report sections,
5+
``--log-cli-level`` and ``--log-file`` output) now sees each record exactly
6+
once, and the previously-missed ``True -> False`` transition on the affected
7+
loggers and their ancestors is now handled as well.

‎src/_pytest/logging.py‎

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -334,16 +334,58 @@ def add_option_ini(option, dest, default=None, type=None, **kwargs):
334334
_HandlerType = TypeVar("_HandlerType", bound=logging.Handler)
335335

336336

337+
class _BoundProxyHandler(logging.Handler):
338+
"""A proxy for a pytest capture handler, bound to one logger.
339+
340+
The proxy forwards records to the real handler only while its logger
341+
currently does not propagate. This is attached (instead of the real
342+
handler) to loggers which were non-propagating when capture started, and
343+
to their ancestors, so that flipping ``Logger.propagate`` during a test
344+
neither duplicates the record (direct handler plus root handler) nor
345+
misses it (#15064, #3697).
346+
"""
347+
348+
__slots__ = ("logger", "real_handler")
349+
350+
def __init__(self, logger: logging.Logger, real_handler: logging.Handler) -> None:
351+
self.logger = logger
352+
self.real_handler = real_handler
353+
super().__init__()
354+
355+
@property
356+
def level(self) -> int:
357+
# Always defer to the real handler, whose level may change after
358+
# attachment (e.g. via caplog.set_level()).
359+
return self.real_handler.level
360+
361+
@level.setter
362+
def level(self, value: int) -> None:
363+
# Only assigned by logging.Handler.__init__(); the level is tracked
364+
# through the real handler instead.
365+
pass
366+
367+
def emit(self, record: logging.LogRecord) -> None:
368+
if not self.logger.propagate:
369+
self.real_handler.handle(record)
370+
371+
337372
# Not using @contextmanager for performance reasons.
338373
class catching_logs(Generic[_HandlerType]):
339374
"""Context manager that prepares the whole logging machinery properly."""
340375

341-
__slots__ = ("attached_loggers", "handler", "level", "orig_level")
376+
__slots__ = (
377+
"attached_loggers",
378+
"attached_proxies",
379+
"handler",
380+
"level",
381+
"orig_level",
382+
)
342383

343384
def __init__(self, handler: _HandlerType, level: int | None = None) -> None:
344385
self.handler = handler
345386
self.level = level
346387
self.attached_loggers: list[logging.Logger] = []
388+
self.attached_proxies: list[tuple[logging.Logger, _BoundProxyHandler]] = []
347389

348390
def __enter__(self) -> _HandlerType:
349391
root_logger = logging.getLogger()
@@ -352,17 +394,30 @@ def __enter__(self) -> _HandlerType:
352394
# Attach to root logger.
353395
root_logger.addHandler(self.handler)
354396
self.attached_loggers.append(root_logger)
355-
# Attach to all non-propagating loggers (won't reach root).
356-
# Note that will miss loggers that *become* non-propagating
357-
# after the `__enter__`. Not worth the trouble for now.
397+
# Attach bound proxy handlers to all non-propagating loggers
398+
# (their records won't reach root) and to their ancestors, so that
399+
# records which *do* reach root after a `propagate` change are only
400+
# handled once. The proxies consult the live `propagate` value per
401+
# record (#15064).
402+
# Note that this still misses loggers (outside those ancestor
403+
# chains) which *become* non-propagating after the `__enter__`.
404+
# Not worth the trouble for now.
405+
proxy_targets: dict[logging.Logger, None] = {}
358406
for logger in root_logger.manager.loggerDict.values():
359407
if (
360408
isinstance(logger, logging.Logger)
361409
and not logger.propagate
362410
and logger is not root_logger
363411
):
364-
logger.addHandler(self.handler)
365-
self.attached_loggers.append(logger)
412+
proxy_targets[logger] = None
413+
parent = logger.parent
414+
while parent is not None and parent is not root_logger:
415+
proxy_targets.setdefault(parent)
416+
parent = parent.parent
417+
for logger in proxy_targets:
418+
proxy = _BoundProxyHandler(logger, self.handler)
419+
logger.addHandler(proxy)
420+
self.attached_proxies.append((logger, proxy))
366421
if self.level is not None:
367422
# Non-propagating loggers still inherit the level (unless a logger
368423
# explicitly set level), so only do this on the root logger.
@@ -382,6 +437,9 @@ def __exit__(
382437
for logger in self.attached_loggers:
383438
logger.removeHandler(self.handler)
384439
self.attached_loggers.clear()
440+
for logger, proxy in self.attached_proxies:
441+
logger.removeHandler(proxy)
442+
self.attached_proxies.clear()
385443

386444

387445
class LogCaptureHandler(logging_StreamHandler):

‎testing/logging/test_fixture.py‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,62 @@ def test_non_propagating_logger(caplog):
472472
result.assert_outcomes(passed=1)
473473

474474

475+
def test_capture_once_when_propagation_enabled_during_test(
476+
pytester: Pytester,
477+
) -> None:
478+
"""A logger which is non-propagating at capture start but enables
479+
propagation during the test must not have its records captured twice
480+
(#15064)."""
481+
pytester.makepyfile(
482+
"""
483+
import logging
484+
485+
logger = logging.getLogger("example")
486+
logger.propagate = False
487+
child_logger = logging.getLogger("example.child")
488+
489+
def test_log_is_captured_once(caplog):
490+
logger.propagate = True
491+
492+
logger.warning("only once")
493+
child_logger.warning("child only once")
494+
495+
assert caplog.messages == ["only once", "child only once"]
496+
"""
497+
)
498+
499+
result = pytester.runpytest()
500+
result.assert_outcomes(passed=1)
501+
502+
503+
def test_capture_once_when_propagation_barrier_moves_to_ancestor(
504+
pytester: Pytester,
505+
) -> None:
506+
"""A child which was non-propagating at capture start and propagates to an
507+
ancestor which becomes the new barrier mid-test is captured exactly once
508+
(#15064)."""
509+
pytester.makepyfile(
510+
"""
511+
import logging
512+
513+
parent = logging.getLogger("mixed.parent")
514+
child = logging.getLogger("mixed.parent.child")
515+
child.propagate = False
516+
517+
def test_barrier_moves(caplog):
518+
child.propagate = True
519+
parent.propagate = False
520+
521+
child.warning("once at new barrier")
522+
523+
assert caplog.messages == ["once at new barrier"]
524+
"""
525+
)
526+
527+
result = pytester.runpytest()
528+
result.assert_outcomes(passed=1)
529+
530+
475531
def test_captures_despite_exception(pytester: Pytester) -> None:
476532
pytester.makepyfile(
477533
"""

‎testing/logging/test_reporting.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,6 +1287,37 @@ def test_log_file():
12871287
assert not list(report.get_sections("Captured stderr call"))
12881288

12891289

1290+
def test_log_propagation_enabled_during_test_captured_once(
1291+
pytester: Pytester,
1292+
) -> None:
1293+
"""Records from a logger which enables propagation during the test appear
1294+
exactly once in the report's captured-log sections (#15064)."""
1295+
pytester.makepyfile(
1296+
"""
1297+
import logging
1298+
1299+
logging.getLogger('foo').propagate = False
1300+
1301+
def test_log_once():
1302+
logging.getLogger('foo').warning("before enabling propagation")
1303+
logging.getLogger('foo').propagate = True
1304+
logging.getLogger('foo').warning("after enabling propagation")
1305+
assert False, "intentionally fail to trigger report logging output"
1306+
"""
1307+
)
1308+
1309+
reprec = pytester.inline_run()
1310+
reports = reprec.getfailures()
1311+
assert len(reports) == 1
1312+
report = reports[0]
1313+
sections = list(report.get_sections("Captured log call"))
1314+
assert len(sections) == 1
1315+
log_text = sections[0][1]
1316+
assert log_text.count("before enabling propagation") == 1
1317+
assert log_text.count("after enabling propagation") == 1
1318+
assert log_text.count("WARNING") == 2
1319+
1320+
12901321
def test_colored_ansi_esc_caplogtext(pytester: Pytester) -> None:
12911322
"""Make sure that caplog.text does not contain ANSI escape sequences."""
12921323
pytester.makepyfile(

0 commit comments

Comments
 (0)