Skip to content

Comments

fix(discovery): TransactionTestCase isolation via toxic queue routing#90

Merged
NikkeTryHard merged 2 commits intomasterfrom
fix/issue-82-transaction-test-isolation
Feb 10, 2026
Merged

fix(discovery): TransactionTestCase isolation via toxic queue routing#90
NikkeTryHard merged 2 commits intomasterfrom
fix/issue-82-transaction-test-isolation

Conversation

@NikkeTryHard
Copy link
Owner

Summary

  • Mark @pytest.mark.django_db(transaction=True) tests as toxic so workers exit after running them
  • Process-death provides DB isolation — no flush/truncate needed in fork-based architecture
  • Close stale connections for transaction tests before returning from isolation setup

Closes #82

How it works

Tests with transaction=True commit directly to the database, bypassing Django's savepoint isolation. Instead of implementing Django's flush command (expensive, complex), we leverage tach's existing toxic queue:

  1. Scanner already parses transaction=True into MarkerInfo
  2. New: has_django_transaction_marker() on RunnableTest checks for this marker
  3. New: Toxicity tagging in main.rs ORs the existing graph-based check with the marker check
  4. Result: Transaction tests → toxic queue → worker exits after test → clean DB from Zygote snapshot

Files Changed

File Change
src/discovery/resolver.rs Add has_django_transaction_marker() + 3 unit tests
src/main.rs OR marker check into both toxicity tagging locations
src/tach_harness.py Close stale connections for transaction=True path

Verification

  • cargo build ✅ | cargo fmt ✅ | cargo clippy -D warnings ✅ | 857/857 tests pass
  • Docker verified

Tests with @pytest.mark.django_db(transaction=True) commit directly to
the database, bypassing savepoint isolation. Mark them as toxic so
workers exit after running them, providing process-level DB isolation
via the fork-based architecture.

Add has_django_transaction_marker() to RunnableTest, wire into both
toxicity tagging locations in main.rs.

Refs #82
Update _apply_django_db_isolation to close stale database connections
even for transaction=True tests before returning. Savepoints are skipped
since these tests are now routed to the toxic queue where process-death
provides isolation.

Closes #82
@coderabbitai
Copy link

coderabbitai bot commented Feb 10, 2026

Warning

Rate limit exceeded

@NikkeTryHard has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 37 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/issue-82-transaction-test-isolation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@NikkeTryHard NikkeTryHard merged commit f0cac23 into master Feb 10, 2026
2 checks passed
@NikkeTryHard NikkeTryHard deleted the fix/issue-82-transaction-test-isolation branch February 10, 2026 03:28
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.

fix(execution): support Django TransactionTestCase flush isolation

1 participant