Repository navigation
Conversation
Circular import analysis
|
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
BenchmarksBenchmark execution time: 2026-10-06 20:12:16 Comparing candidate commit d69c01c in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.
|
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🔗 Commit SHA: d69c01c | Docs | View more details | Give us feedback! |
cf30d48 to
8bf5751
Compare
gnufede
changed the title
Co-authored-by: gnufede <412857+gnufede@users.noreply.github.com>
…estores The guarded stack exits from the previous commit prevent the crash caused by stack ownership moving between execution contexts, but a second failure mode remains: value-based context restoration (e.g. asgiref's ``_restore_context``, used by Django's async test support via ``async_to_sync``) compares ContextVar values with ``!=``. Plain list stacks compare by value, so two distinct stacks holding equal entries compare equal and the restore is masked: the asyncio task keeps the loop-thread wrapper's stack, the test body's coverage data is recorded into the wrapper's entry, and the per-test collector reads its own (now empty) entry. The final restore, running before the executor thread join, then replaces the caller's stack with the wrapper's, stranding the data: async tests silently report no coverage (and can never be selected for skipping under ITR). Make the per-context stacks compare by identity so value-based restores propagate the correct stack: distinct stacks are never equal (restores are no longer masked, so the task starts with the caller's stack and coverage is attributed to the right test), and a context already holding the exact stack object ignores further restores of it (no forced swap, push/pop pairing stays balanced). Verified against Django's test suite (pytest + CI Visibility coverage, asgiref ``async_to_sync``/``sync_to_async``): both line-level and file-level coverage now complete the full run (2035 tests) with no internal errors, and async tests report non-empty coverage again. Co-authored-by: Claude <noreply@anthropic.com>
The stacks hold different element types (defaultdict[str, CoverageLines] for lines, set[str] for files), so parameterize the generic base with Any to satisfy mypy's type-arg check without duplicating the class.
8ac6dc7 to
7f2ce0a
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f2ce0a543
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
On Python 3.14+, sys.monitoring callbacks run in a snapshot context and fall back to the thread-local coverage state when they cannot observe ContextVar changes. When a collector entered in a copied context is exited from a context whose stack top is a different collector, the early return left _tls_coverage pointing at the completed collector, so subsequent instrumented code in that thread was recorded into the stale entry instead of the active collector. Re-sync the fallback to the current stack top (or clear it) before returning, mirroring what the pop path already does.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c084f7c30a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… entries A context copied while a collector was active (eg. a task scheduled by a module imported inside a test) keeps a reference to a stack that still contains that collector's entry after it completes. Coverage recorded in such a context was written into the completed collector's entry — one that after_import() had already consumed — instead of the enclosing active collector, so the data was silently lost. Mark each collector's entries as closed when it exits, and make the context resolvers (and the Python 3.14+ TLS fallback) walk down the stack to the nearest entry that is still open, attributing new coverage to the closest active collector instead of an orphaned entry.
The coverage module is private (nothing under docs/ renders it), so docstrings and comments should read as plain text per the repository convention in AGENTS.md: drop the rST double-backtick literals and write the names as-is.
2 participants
Summary
Since #19956, pytest tests that execute user code through
asgiref'sasync_to_sync/sync_to_async(Django async test support,sync_to_asyncORM calls) with CI Visibility coverage enabled abort the whole session:
A large suite dies at its first async test (Django: ~282 of 2035); both
line-level and file-level coverage modes are affected. #19956 stores
per-context coverage data in
ContextVars holding plainliststacks, andplain lists break the
__enter__/__exit__push/pop pairing in twoindependent ways when contexts are copied or restored — this branch fixes
both, one per commit.
Failure mode 1 — stacks shared by reference into copied contexts
copy_context()shares ContextVar objects by reference, so a collectorexited in a different context (threading wrappers, async bridges) pops the
wrong entry — or an empty list (
IndexError).Fix (
88884502bb"guard coverage stack exits"): copy-on-write stackper context on
__enter__;__exit__pops only if the stack top is thecollector's own entry;
get_covered_lines()reads the collector's ownentry, so results are correct regardless of where it exits.
Failure mode 2 — value-based context restoration masks/forces stack swaps
asgiref's
_restore_contextrestores context values by comparing them with!=, and plain lists compare by value: the initial restore into theasyncio task is masked (two empty stacks compare equal), so the test body's
coverage lands in the loop-thread wrapper's entry; the final restore then
swaps the wrapper's stack into the pytest context, whose pop double-pops the
emptied stack →
IndexError→ session aborted. Even when guarded away, thedata stays stranded: async tests silently report empty coverage and can
never be safely skipped under ITR — guarding mode 1 alone does not fix this.
Fix (
a89ce3907f"compare coverage stacks by identity for contextrestores"): stacks are now
_ContextStackobjects comparing byidentity, so restores are never masked (coverage is attributed to the
right test), become no-ops when the context already holds that exact stack
(pops stay balanced), and genuine propagation across copies still works.
Regression tests
Four new tests, two per mode, in
tests/coverage/test_coverage.pyandtests/coverage/test_coverage_asgiref_context_propagation.py. The key onehand-rolls the value-based restore contract across threads with no new
dependencies; the other drives the real asgiref flow (skipif-guarded).
Both fail on
mainwith the exact productionIndexError, and the asgireftest additionally fails with empty coverage on the guarded-exits-only
fix, discriminating mode 2. The entire pre-existing coverage suite (121
tests) passes on unfixed
main— nothing covered context propagationacross threads.
PR by Bits - View session in Datadog
Comment @DataDog to request changes