Re-arm the MySQL data-migration fixture in CI — the dedupe DML has no automated coverage on MySQL #627

New Issue

2026-07-25T21:41:46+02:00

timothy commented

2026-07-25 21:41:46 +02:00

Split out of #491 (PR #613) after three failed attempts to make the fixture deterministic in CI. The coverage gap is real and demonstrated; the blocker is test-harness reliability, not the value of the gate.

The gap

CI's migrations job only ever applies migrations to a fresh, empty database, so it executes zero rows of any data-migration logic. LibraryFolderDedupeMigrationTests is parameterized over both providers from one fixture, but only the SQLite half runs anywhere automatically.

That is not theoretical. Two MySQL-only collation defects escaped exactly this gate during #491 and were caught only by hand-run servers:

Case-insensitive grouping (fixed by an explicit COLLATE).
PAD SPACE — utf8mb4_bin treats /media/Foo and /media/Foo as equal, so the dedupe would have irreversibly deleted a distinct folder row. Fixed with CONVERT(Path USING binary).

Both were in a migration that deletes user rows. A future edit to that migration, or a new dedupe migration copied from it, currently has no automated backstop on MySQL.

Why it was removed rather than shipped

The fixture proved non-deterministic in CI across three attempts, each with a distinct root cause:

Stale pooled session after DROP DATABASE. MySqlConnector hands back a live pooled session whose default schema is gone, so the failure depends on whether the pool reuses a session or opens a fresh one (which fails 1049). Also measured: a per-test database name leaked one connection pool per test — ~1/iteration, Too many connections at 400, and a leak-saturated server turned a 1m21s run into 13 minutes. "Slow" and "flaky" were the same defect.
Lost isolation. Collapsing to a single fixed database name fixed the leak but gave up per-test isolation, producing Duplicate entry '1' for key 'LibraryPath.PRIMARY' — the second test seeded on top of the first's rows in 539ms because MigrateAsync no-opped against an already-current __EFMigrationsHistory. Verified afterwards that unique-name plus ClearPoolAsync gives leak/iter = 0, so isolation was free and collapsing the name was solving an already-solved problem.
Connect-before-create. Restoring unique names brought back Unknown database 'etv491_<guid>', now with TearDown reporting the database never existed at all — so something opens a connection to it before MigrateAsync creates it.

An intermittently-red gate is worse than no gate: it trains everyone to re-run instead of read, which is precisely how the two original defects escaped. So it was removed rather than merged in an unreliable state.

What already exists to build on

LibraryFolderDedupeMigrationTests is parameterized [TestFixture(TestProvider.Sqlite)] + [TestFixture(TestProvider.MySql)] over one shared body — the property that stops the providers silently diverging. Preserve it.
The MySQL half is opt-in via ETV_TEST_MYSQL_CONNECTION and skips visibly without it; ETV_REQUIRE_MYSQL_TESTS=1 turns that skip into a hard failure, which is the fail-closed behaviour CI needs.
The removed CI step is documented in a comment in .gitea/workflows/docker-build.yml in the migrations job, next to where it belongs.
The mysql:8.4 service is already declared by that job, so no new infrastructure is needed.

Done-when

The dedupe fixture runs against live MySQL in CI, fail-closed (a missing server fails, never silently skips)
Determinism demonstrated over ≥10 consecutive runs and across test orders (each test alone, and both orders) — order-independence is the axis that hid failure #2, and single-order evidence is what let two of the three attempts look green
No [Retry] or re-run-on-failure anywhere in the mechanism
Connection-pool/thread count and leftover etv491% schemas verified flat across those runs
The red-when-broken control re-confirmed after the fix (restore COLLATE utf8mb4_bin, watch the trailing-space sibling row disappear) — a fix that quietly neuters the assertion is the main risk
One shared fixture body across both providers preserved
Adversarial review passed

Ref: #491, PR #613. Related decision: scan.libraryfolder-unique-identity.

Split out of #491 (PR #613) after three failed attempts to make the fixture deterministic in CI. The coverage gap is **real and demonstrated**; the blocker is test-harness reliability, not the value of the gate. ## The gap CI's `migrations` job only ever applies migrations to a **fresh, empty** database, so it executes **zero rows** of any data-migration logic. `LibraryFolderDedupeMigrationTests` is parameterized over both providers from one fixture, but only the SQLite half runs anywhere automatically. That is not theoretical. Two MySQL-only collation defects escaped exactly this gate during #491 and were caught only by hand-run servers: 1. Case-insensitive grouping (fixed by an explicit `COLLATE`). 2. **PAD SPACE** — `utf8mb4_bin` treats `/media/Foo` and `/media/Foo ` as equal, so the dedupe would have **irreversibly deleted** a distinct folder row. Fixed with `CONVERT(Path USING binary)`. Both were in a migration that **deletes user rows**. A future edit to that migration, or a new dedupe migration copied from it, currently has no automated backstop on MySQL. ## Why it was removed rather than shipped The fixture proved non-deterministic in CI across three attempts, each with a distinct root cause: 1. **Stale pooled session after `DROP DATABASE`.** MySqlConnector hands back a live pooled session whose default schema is gone, so the failure depends on whether the pool *reuses* a session or opens a *fresh* one (which fails 1049). Also measured: a per-test database name leaked one connection pool per test — ~1/iteration, `Too many connections` at 400, and a leak-saturated server turned a 1m21s run into 13 minutes. "Slow" and "flaky" were the same defect. 2. **Lost isolation.** Collapsing to a single fixed database name fixed the leak but gave up per-test isolation, producing `Duplicate entry '1' for key 'LibraryPath.PRIMARY'` — the second test seeded on top of the first's rows in 539ms because `MigrateAsync` no-opped against an already-current `__EFMigrationsHistory`. Verified afterwards that unique-name **plus** `ClearPoolAsync` gives leak/iter = 0, so isolation was free and collapsing the name was solving an already-solved problem. 3. **Connect-before-create.** Restoring unique names brought back `Unknown database 'etv491_<guid>'`, now with TearDown reporting the database never existed at all — so something opens a connection to it before `MigrateAsync` creates it. An intermittently-red gate is **worse than no gate**: it trains everyone to re-run instead of read, which is precisely how the two original defects escaped. So it was removed rather than merged in an unreliable state. ## What already exists to build on - `LibraryFolderDedupeMigrationTests` is parameterized `[TestFixture(TestProvider.Sqlite)]` + `[TestFixture(TestProvider.MySql)]` over **one shared body** — the property that stops the providers silently diverging. Preserve it. - The MySQL half is opt-in via `ETV_TEST_MYSQL_CONNECTION` and skips **visibly** without it; `ETV_REQUIRE_MYSQL_TESTS=1` turns that skip into a hard failure, which is the fail-closed behaviour CI needs. - The removed CI step is documented in a comment in `.gitea/workflows/docker-build.yml` in the `migrations` job, next to where it belongs. - The `mysql:8.4` service is already declared by that job, so no new infrastructure is needed. ## Done-when - [ ] The dedupe fixture runs against live MySQL in CI, fail-closed (a missing server fails, never silently skips) - [ ] Determinism demonstrated over ≥10 consecutive runs **and** across test orders (each test alone, and both orders) — order-independence is the axis that hid failure #2, and single-order evidence is what let two of the three attempts look green - [ ] No `[Retry]` or re-run-on-failure anywhere in the mechanism - [ ] Connection-pool/thread count and leftover `etv491%` schemas verified flat across those runs - [ ] The red-when-broken control re-confirmed **after** the fix (restore `COLLATE utf8mb4_bin`, watch the trailing-space sibling row disappear) — a fix that quietly neuters the assertion is the main risk - [ ] One shared fixture body across both providers preserved - [ ] Adversarial review passed Ref: #491, PR #613. Related decision: `scan.libraryfolder-unique-identity`.

timothy added the ci-cd priority: low labels 2026-07-25 21:41:46 +02:00

timothy referenced this issue from a commit

2026-07-25 21:43:20 +02:00

ci(491): split the MySql dedupe fixture out of CI, tracked by #627

timothy referenced this issue

2026-07-25 21:43:37 +02:00

fix(491): unique index on LibraryFolder(LibraryPathId, PathHash) + tolerate concurrent insert #613

timothy referenced this issue

2026-07-25 22:02:17 +02:00

LibraryFolder has no unique index on (LibraryPathId, Path) — concurrent GetOrAddFolder can insert duplicates #491

timothy referenced this issue

2026-07-27 22:08:19 +02:00