Skip to content

Remove hook-call monitors independently of undo order - #814

Open
rupayon123 wants to merge 3 commits into
pytest-dev:mainfrom
rupayon123:fix/monitor-undo-order-20261028
Open

rupayon123 wants to merge 3 commits into
pytest-dev:mainfrom
rupayon123:fix/monitor-undo-order-20261028

Conversation

@rupayon123

@rupayon123 rupayon123 commented Oct 28, 2026 •

Copy link
Copy Markdown

Undoing an earlier hook-call monitor currently removes later monitors, and later undo calls can restore callbacks that were already removed. Keep a persistent execution chain so each undo removes only its own monitor, repeated undo stays harmless, and in-flight calls retain their original before/after pairing.

The lifetime regression invokes active callbacks before checking their release. Undo traversal now expresses the internal invariant that every active token remains in the chain; directly replacing the private _inner_hookexec while monitors are active fails that assertion. Existing public tracing/monitoring lifecycles, custom original executors, nested calls and result/exception handling remain covered. Includes the numbered changelog fragment.

Validation on macOS arm64:

  • All 260 repository tests, 68 focused tests and all 10 pinned pre-commit hooks pass with the frozen lockfile.
  • Every changed line and branch is covered locally; combined coverage is 99.9741%, versus 99.9727% on pristine base. The same old _caller.py branch remains uncovered locally.
  • Native lifecycle checks cover all 120 removal orders for five monitors, custom original-executor identity, nested calls and callback collection. Independent review adds public normal/first-result, self-undo/reinstall and tracing/result-change scenarios.
  • The hosted benchmark failure reproduces during fixture setup on both unchanged base and this branch with pytest 9.2.0 and pytest-benchmark 5.3.0: the fixture accesses the removed Function._nodeid. Local reproduction uses Python 3.10.17; the hosted job uses 3.10.22 on Linux. No benchmark, dependency, coverage threshold or configuration was changed.

Hosted checks for the latest repair still need verification. No cross-thread registration/undo guarantee is added.

@rupayon123
rupayon123 marked this pull request as ready for review October 28, 2026 12:49
@rupayon123

Copy link
Copy Markdown
Author

I updated the lifetime test to call both monitors before checking that their callbacks are released, and simplified undo around the active-token invariant. The full 260 tests, 68 focused tests and all pinned hooks pass. Every changed line and branch is covered locally; hosted coverage for this commit still needs to run.

The benchmark failure also occurs on unchanged base with the job's pytest 9.2.0 and pytest-benchmark 5.3.0 versions: the fixture accesses the removed Function._nodeid before the benchmark body starts. I left benchmark dependencies and configuration unchanged. The PR description now records that baseline comparison and the private execution-chain invariant.

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.

1 participant