From 396b860d59f3678e9d9dcd8013c4ceea03b3bf8b Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 8 Aug 2026 21:07:01 -0400 Subject: [PATCH] test(migrate-ts): green the migrate-ts-pg lane, red on every release tag The `migrate-ts-pg` job in integration-tests.yml has failed on EVERY release tag back to at least v0.20.0 -- the same 6 tests on v0.21.0 and v0.21.1. It went unnoticed structurally, not carelessly: the lane runs ONLY on `v*` tag push, so the red arrives AFTER publishing, and the same tests `describe.skip` silently without MIGRATE_TS_PG_URL, so local and PR runs look green. A release gate that is permanently red provides no signal, which is how the #279 serial-PK adoption bug reached a release. Both root causes are test-side; no product defect. 1. Sourceless fixtures (4 of 6 failures). `postgres-lenient-inet` and `pg-adversarial-fixes` declared an `object.entity` with NO `source.rdb` child. Persistability derives from a declared/inherited `source.*` (#249), so those entities are non-persistable: no CREATE TABLE is emitted, the table never exists, and the assertions fail -- then cascade as `relation "endpoints"/"readings" does not exist`. Proven, not inferred: adding `source.rdb` alone takes each file from all-fail to all-pass. 2. Stale CHECK-expression spacing (2 of 6). The expectations still carried `'open', 'closed'` with spaces after the commas, but `checkExprs` runs `normalizeCheckExpr`, which deliberately collapses comma spacing outside literals. The normalizer is right; the expectations predate it. Verified against a real Postgres 16: the full integration suite goes 124 pass / 6 fail -> 130 pass / 0 fail. The no-PG path still skips cleanly (688 pass / 19 skip / 0 fail), so PR runs are unaffected. Deliberately NOT changed: the lane's tag-only trigger. Making it run on merge is a workflow policy decision for the maintainer, not a test fix -- but until it does, this lane can silently rot again. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015TqsuDye2SfXGf43vuoD3n --- .../migrate-ts/test/integration/lifecycle-pg.test.ts | 8 ++++---- .../test/integration/pg-adversarial-fixes.test.ts | 1 + .../test/integration/postgres-lenient-inet.test.ts | 1 + 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/server/typescript/packages/migrate-ts/test/integration/lifecycle-pg.test.ts b/server/typescript/packages/migrate-ts/test/integration/lifecycle-pg.test.ts index b3aedecd9..d137ddd31 100644 --- a/server/typescript/packages/migrate-ts/test/integration/lifecycle-pg.test.ts +++ b/server/typescript/packages/migrate-ts/test/integration/lifecycle-pg.test.ts @@ -159,7 +159,7 @@ d("migrate-ts lifecycle against real Postgres", () => { expect(orders1.indexes.some((i) => i.unique && i.columns.includes("email"))).toBe(true); // enum + numeric checks present expect(checkExprs(orders1)).toEqual( - ["qty >= 1", "status in 'open', 'closed'"].sort(), + ["qty >= 1", "status in 'open','closed'"].sort(), ); // ---- v2: evolve --------------------------------------------------------- @@ -189,7 +189,7 @@ d("migrate-ts lifecycle against real Postgres", () => { expect(orders2.foreignKeys.some((f) => f.refTable === "lc_customers")).toBe(true); // evolved checks const c2 = checkExprs(orders2); - expect(c2).toContain("status in 'open', 'closed', 'cancelled'"); + expect(c2).toContain("status in 'open','closed','cancelled'"); expect(c2).toContain("qty >= 1 and qty <= 1000"); // ---- rollback v2 → v1 (down) -------------------------------------------- @@ -203,7 +203,7 @@ d("migrate-ts lifecycle against real Postgres", () => { expect(colNames(ordersBack)).toEqual(["email", "id", "qty", "status"]); // note/customerId dropped expect(ordersBack.foreignKeys).toHaveLength(0); // fk dropped expect(checkExprs(ordersBack)).toEqual( - ["qty >= 1", "status in 'open', 'closed'"].sort(), // original checks restored + ["qty >= 1", "status in 'open','closed'"].sort(), // original checks restored ); // ---- idempotency: re-diff v1 against the live (rolled-back) DB ---------- @@ -326,7 +326,7 @@ d("migrate-ts lifecycle against real Postgres", () => { expect(posts.foreignKeys.some((f) => f.refTable === "gf_users")).toBe(true); expect(comments.foreignKeys.map((f) => f.refTable).sort()).toEqual(["gf_posts", "gf_users"]); // enum + numeric checks + unique indexes survived the round-trip - expect(checkExprs(gf.find((t) => t.name === "gf_users")!)).toContain("role in 'admin', 'editor', 'viewer'"); + expect(checkExprs(gf.find((t) => t.name === "gf_users")!)).toContain("role in 'admin','editor','viewer'"); expect(checkExprs(posts).some((e) => e.includes("views >= 0"))).toBe(true); expect(gf.find((t) => t.name === "gf_tags")!.indexes.some((i) => i.unique && i.columns.includes("name"))).toBe(true); diff --git a/server/typescript/packages/migrate-ts/test/integration/pg-adversarial-fixes.test.ts b/server/typescript/packages/migrate-ts/test/integration/pg-adversarial-fixes.test.ts index 14cdffb2b..0bab0fa84 100644 --- a/server/typescript/packages/migrate-ts/test/integration/pg-adversarial-fixes.test.ts +++ b/server/typescript/packages/migrate-ts/test/integration/pg-adversarial-fixes.test.ts @@ -39,6 +39,7 @@ const META = JSON.stringify({ "object.entity": { name: "Reading", children: [ + { "source.rdb": { "@table": "readings" } }, { "field.long": { name: "id" } }, // Bug 6 — the scalar @isArray fan-out beyond string/uuid. { "field.int": { name: "counts", isArray: true } }, diff --git a/server/typescript/packages/migrate-ts/test/integration/postgres-lenient-inet.test.ts b/server/typescript/packages/migrate-ts/test/integration/postgres-lenient-inet.test.ts index cda70cef0..3346ba3f5 100644 --- a/server/typescript/packages/migrate-ts/test/integration/postgres-lenient-inet.test.ts +++ b/server/typescript/packages/migrate-ts/test/integration/postgres-lenient-inet.test.ts @@ -35,6 +35,7 @@ const META = JSON.stringify({ "object.entity": { name: "Endpoint", children: [ + { "source.rdb": { "@table": "endpoints" } }, { "field.long": { name: "id" } }, { "field.inet": { name: "strictIp" } }, { "field.inet": { name: "lenientIp", "@lenient": true } },