test: retire hand-rolled migration lists, grep-ban drift (LC-204) #266
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/lc-204-retire-handrolled-migration-lists"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Retires the hand-rolled
include_str!("../migrations/...)migration lists in the integration test suite. Each pinned a stale migration subset that silently went out of date every time a migration was added, surfacing later as "no such column" 500s (hit twice in recent PRs: 0054/0055 in LC-78, 0056 in avatar-proxy; plus 0052/0053 that were never added to any list pre-LC-78). The cure: build every test pool via the drift-immunecommon::auth_pool()/common::chat_pool()/common::settings_pool()helpers, which run the full set viasqlx::migrate!so new migrations land in every test automatically.Audit (gated the whole change)
The ticket estimated 18 files; the actual population is 35. Every file was classified before any conversion:
common::*_pool()migration_enclaves.rs— stays hand-rolledmigration_enclaves.rsis the one file the audit existed to catch: it is a migration-BEHAVIOR test (asserts the 0009 enclaves data-migration's effect — General created, rooms moved, membership-backfill-is-separate, partial-unique-owner-index enforced), not a schema consumer. Converting it to the fullsqlx::migrate!set would silently re-point its assertions from "0009 produced exactly this state" to "the full-migration end-state happens to have these properties" — a different test that could stay green while no longer catching a 0009 regression. It keeps its explicit list, carries a justifying comment, and is the sole grep-ban allow-list entry.Conversion mechanics
Mechanical and uniform: each pool-builder's hand-rolled list → the matching
common::*_pool().await, function name/signature preserved so call sites and all caller-side harness setup (create_user,backfill_general_membership,totp_enabled=1, room seeding) are untouched. Verified drop-in: the helpers are pure pool-builders (migrate + returnSqlitePool), and harness setup always lives AFTER the pool build, never inside it.AppStatewiring unchanged.db_mentions/db_enclave: finished their already-half-converted auth halves.Diff signature: 116 insertions, 2269 deletions across 35 files — deleting lists, adding helper calls.
This is also the cleanup for the pre-existing rot: 0052/0053 (never listed), the accidental 0034-0040 gaps, and 0057 are now present in every test by construction.
Drift prevention (structural, not conventional)
tests/lc204_no_handrolled_migrations.rswalkstests/and fails the build on anyinclude_str!("../migrations/)outside the allow-list:migrations_test_data/.tests/migration_enclaves.rs), not basename — a futuretests/foo/migration_enclaves.rscan't inherit the exemption.Item-5 triage (newly-surfaced failures)
Converting partial→full runs every previously-skipped migration, which can surface breakage the stale lists were hiding. Triage question per failure: does fixing it change what the test ASSERTS (load-bearing → split to follow-up) or only what it FEEDS IN (test-data fix → in-PR commit)?
Result: zero genuine conversion failures. Both suite failures are pre-existing, confirmed identical on clean
main:lc77_webhook_render_fixture(4 tests)routes_uploads::other_user_cannot_fetch_orphan_uploadNo test-data fixes were needed and no hidden load-bearing-partials surfaced beyond
migration_enclaves.rs.Test plan
./dev/cargo check -p lets-chat-server --testsclean; exactly one hand-rolled list remains (migration_enclaves.rs, the deliberate exemption).just test(standalone): 123 binaries, only the 2 pre-existing failures above (both confirmed on clean main).just test-saas: 59 binaries (standalone-gated files correctly skip), only the pre-existinglc77fixture failure.lc204_no_handrolled_migrations): 2/2 pass — the ban + the sanity meta-test.cargo fmt --allapplied;cargo clippy --testsclean.Discovered finding to file separately
lc77_webhook_render_fixture's 4 golden-fixture tests fail on cleanmain(not caused by this PR). The rendered webhook/email-inbox message HTML diverged from the committed fixtures — likely an un-regenerated fixture after a template edit. NeedsFIXTURE_WRITE=1regeneration (after confirming the new render is correct) under its own ticket.Structural drift-prevention so the converted-away pattern can't return: lc204_no_handrolled_migrations walks tests/ and fails on any include_str!("../migrations/) outside the allow-list. Anchored literal (opening quote + trailing slash) so it can't false-positive on a sibling dir; full-path allow-list so a future tests/foo/migration_enclaves.rs can't inherit the exemption by basename. A sanity meta-test asserts every allow-list entry exists AND still contains the pattern, so the exemption can't silently defang. The allow-list is the load-bearing-partial registry: an entry is a DECLARATION (this file is a migration-behavior test), not a bypass. migration_enclaves.rs is the sole entry; it asserts the 0009 enclaves data-migration's effect, so converting it to the full sqlx::migrate! set would silently re-point its assertions. Carries a justifying comment.