Skip to content

Sweep stale test databases once per test process in pgtest - #66

Merged
SirLouen merged 3 commits into
mainfrom
feat/65-sweep-once
Oct 10, 2026
Merged

SirLouen merged 3 commits into
mainfrom
feat/65-sweep-once

Conversation

@SirLouen

@SirLouen SirLouen commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Closes #65

What

  • pgtest.NewSwept sweeps the stale test databases of a server at most once per test process and per address, then hands out a fresh database as New does. Tests that start while the sweep runs wait for it.
  • A failed sweep, such as one whose role lacks the rights Sweep needs, is logged on the test that ran it, and every test still gets its database. An age of zero or less fails the test.
  • Several test binaries sweeping one server at once stay safe, because Sweep keeps a database in use or younger than the age, and skips one another sweep dropped first.

Why

Downstream applications need their stale test databases cleared without a hand step, from one place instead of a copy in each application.

Testing Instructions

Run the testing toolchain.

Summary by CodeRabbit

  • New Features
    • Added an option to clean up stale test databases before creating a new one. Cleanup runs once per server address and scope during a test process.
    • If cleanup fails, the error is logged and database creation continues.
    • Cleanup requires a positive age threshold.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 36 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 2ecdc6cd-7a16-402c-a635-353a48f14930

📥 Commits

Reviewing files that changed from the base of the PR and between aaae717 and aa27527.


📒 Files selected for processing (1)
  • dbkit/postgres/pgtest/swept_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 303fdd2e-df17-4a8f-adf5-77fb459d65c9


📥 Commits

Reviewing files that changed from the base of the PR and between 7d8973f and aaae717.



📒 Files selected for processing (4)
  • dbkit/postgres/CHANGELOG.md
  • dbkit/postgres/pgtest/export_test.go
  • dbkit/postgres/pgtest/swept.go
  • dbkit/postgres/pgtest/swept_test.go


Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.




📝 Walkthrough
📝 Walkthrough

Walkthrough

Adds pgtest.NewSwept to sweep stale test databases once per process and address before creating a database. Sweep errors are logged without stopping the test. The helper rejects non-positive ages.

Changes

pgtest stale-database sweeping

Layer / File(s) Summary
Sweep tracking and database creation
dbkit/postgres/pgtest/swept.go, dbkit/postgres/pgtest/export_test.go
Adds process-wide tracking by address and scope. NewSwept delegates to the helper, which sweeps once per key, logs sweep errors, and then creates a database.
Sweep behavior validation and documentation
dbkit/postgres/pgtest/swept_test.go, dbkit/postgres/CHANGELOG.md
Tests sweep timing by address, handling of sweep failures, and rejection of non-positive ages. The changelog describes NewSwept behavior.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant NewSwept
  participant newSwept
  participant pgtestdb
  participant New
  Caller->>NewSwept: address, age, migrator
  NewSwept->>newSwept: use empty scope
  newSwept->>pgtestdb: sweep once for address and scope
  pgtestdb-->>newSwept: sweep result
  newSwept->>New: create database
  New-->>Caller: database address
Loading


Merge Risk: ⚪ Minimal · up to aaae7

No actionable merge-blocking issue was identified. The testing toolchain should still be run as a normal pre-merge check.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding behavior to sweep stale test databases once per test process in pgtest.
Linked Issues check Passed Issue #65 is active and directly linked. NewSwept validates a positive age, uses a process-wide sync.Map of sync.Once values keyed by address and scope, waits for an in-progress sweep, logs swee…
Out of Scope Changes check Passed The changes stay within issue #65. swept.go implements the requested helper. swept_test.go and export_test.go support automated verification. The changelog documents the new helper. No unrelated…


Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (1 skipped: 1 unsupported.)




✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR



🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@greptile-apps

greptile-apps Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium impact] No blocking findings remain. The changes are safe to merge based on the reviewed scope.

T-Rex evidence

Evidence from the check

  • The executed shell script inspected only the selected diff and available database programs, documenting the validation scope.

Command output from the check

  • The inspection captured the added test, Go version, and absence of PostgreSQL executables, supporting isolated validation.

Evidence from the check

  • The executed Python script extracted revision-specific source and ran five bounded Go tests, preserving the exact reproduction procedure.

Evidence from the check

  • The executed shell script ran the generator and captured its output and exit status, recording the orchestration.

Command output from the check

  • The orchestration capture records all five subprocess exit statuses, showing that both head reproductions failed as expected.

Evidence from the check

  • This generated Go source contains the previous helper and isolated fixtures without the absent overlapping-caller test, establishing the before scope.

Command output from the check

  • The previous-revision harness passed its fixture control, showing that the added test was absent.

Evidence from the check

  • This generated Go source runs the unchanged added test and helper against isolated fixtures and the real worker implementation, preserving the polling reproduction.

Command output from the check

  • The worker failed through real NewSwept code and the stack remained in sweepAwaitDrops until the two-second timeout, confirming the polling stall.

Evidence from the check

  • This generated Go source uses the same DROP-visible fixture without the absent overlapping-caller test, establishing the receive comparison scope.

Command output from the check

  • The previous-revision harness passed under the DROP-visible fixture, showing that the added receive path was absent.

Evidence from the check

  • This generated Go source runs the unchanged added test with DROP observation enabled, allowing execution to reach the unbounded first-result receive.

Command output from the check

  • Both workers exited through real failure paths and rollback completed before the first-result receive timed out, confirming the receive stall.

Evidence from the check

  • This generated Go source supplies deferred completion to the unchanged helper after a real worker failure, testing the existing early-exit branch.

Command output from the check

  • The helper reported worker completion before DROP and failed in 0.004 seconds without a timeout, supporting completion notification as the missing safeguard.

View artifacts

Summary

Adds pgtest.NewSwept to sweep stale databases once per address in each test process before creating fresh databases.

  • Tests sweep stale databases once before making fresh ones.

Reviews (2) · Last reviewed commit: "test(dbkit/postgres): show a later NewSw..." · Reviewed by Greptile

Comment thread dbkit/postgres/pgtest/swept_test.go
@SirLouen
SirLouen merged commit de299e6 into main Oct 10, 2026
34 checks passed
@SirLouen
SirLouen deleted the feat/65-sweep-once branch October 10, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sweep stale test databases once per test process in pgtest

1 participant