Skip to content

fix: undo hookcall monitors independently - #816

Closed
mikegit123git wants to merge 1 commit into
pytest-dev:mainfrom
mikegit123git:fix-815-independent-monitor-undo
Closed

mikegit123git wants to merge 1 commit into
pytest-dev:mainfrom
mikegit123git:fix-815-independent-monitor-undo

Conversation

@mikegit123git

Copy link
Copy Markdown

Closes #815

Problem

add_hookcall_monitoring() currently assumes LIFO removal.

Undoing an older monitor removes newer monitors, while later undo operations can restore previously removed monitors.

The same issue affects enable_tracing().

There is no known real-world failure reported so far.

Fix

  • Maintain an ordered list of active monitor pairs.
  • Remove individual monitors by identity.
  • Rebuild the hook execution chain after changes.
  • Make repeated undo calls harmless.
  • Preserve callbacks for hook calls already in flight.
  • Restore the original executor when all monitors are removed.

This follows the smaller list-based approach suggested in #815.

Regression tests

Added tests for:

  • Non-LIFO monitor removal
  • Repeated undo
  • Existing LIFO behavior
  • Undo during hook execution
  • Custom executor restoration
  • Interactions with enable_tracing

Added changelog/815.bugfix.rst.

Verification

Reproduced the original bug on Linux.

After the fix:

  • Full pytest suite: 233 passed
  • Ruff: passed
  • mypy: passed
  • Pre-commit: passed
  • Git whitespace check: passed

AI assistance

ChatGPT assisted with the implementation, regression tests, and PR description.

The reproducer, full test suite, and pre-commit checks were executed in a Linux development environment.

The contributor will personally review and respond to maintainer feedback.

Copy link
Copy Markdown
Member

🤖 Written by Claude Opus 5.5 via Claude Code for the pluggy maintainers; I prompted it, it did the work, I read it.

Thanks, but I'm closing this. #815 was a note from my own review of #814, and I'm already working on a different fix: monitors become controllers inside _multicall, stacked on #710, which also drops _inner_hookexec. The nested-closure rebuild here doesn't fit that, and a test here reaches into _inner_hookexec.

This PR restates the "possible fix" text from the issue, generated with ChatGPT, and went up two hours after the issue with no discussion first. That isn't the kind of contribution our AI policy asks for: https://github.com/pytest-dev/pytest/blob/main/CONTRIBUTING.rst. If you want to help with an issue, please comment on it first and say what you understand and what you plan to do.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add_hookcall_monitoring undo breaks when called out of LIFO order

2 participants