Skip to content

Commit 9a170c6

Browse files
dmealingclaude
andcommitted
test(migrate-ts): close QA coverage gaps (introspect helpers, version gating, defaults, rename boundary)
Follow-on to the migration QA audit — closes the highest-value coverage gaps the audit surfaced. All test-only (no behavior change); confirmed the audited logic is correct as it stands. - introspect-postgres-helpers: pure unit tests for pgTypeToSqlType + parsePgDefault (exported precisely for DB-free testing; previously only the MIGRATE_TS_PG_URL-gated round-trip exercised them, so pg-mem runs had zero coverage of the --from-db type/default mapping). - emit-sqlite-version-gating: pins the native-vs-recreate thresholds — RENAME COLUMN >= 3.25.0, DROP COLUMN >= 3.35.0 — at the exact boundary, plus unparseable/absent version → treated as modern. - diff-column-default: the change-column-default from/to payload matrix (literal swap, add, remove, expr-vs-literal kind, equal → no change). - diff.test: strengthened five `.toBeDefined()`-only assertions to assert the change payload (kind/table/column/index/fk/view), so a contents regression can't pass. - diff-rename-heuristic: the table-rename Jaccard boundary (== 0.8 inclusive vs 0.75 excluded) and a table-level 'abort' throw. - check-expr normalize: a guard that the IN↔ANY fold does not false-positive on a quoted literal containing the words "any array". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 63fb64e commit 9a170c6

6 files changed

Lines changed: 323 additions & 6 deletions

File tree

server/typescript/packages/migrate-ts/test/check-evolution/normalize.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,17 @@ describe("normalizeCheckExpr", () => {
3535
// but PG's post-literal `::text` cast (enum form) is still stripped
3636
expect(normalizeCheckExpr("'open'::text")).toBe("'open'");
3737
});
38+
test("the IN↔ANY fold does not false-positive on a literal containing 'any array'", () => {
39+
// The fold is anchored on `= any array` (the shape only PG's ARRAY rewrite
40+
// produces after bracket-stripping). A quoted literal that merely contains
41+
// the words must be left intact — a stray quote breaks the `=…any…array`
42+
// adjacency, so there is no `in` rewrite.
43+
expect(normalizeCheckExpr("note = 'pick any array item'")).toBe("note = 'pick any array item'");
44+
expect(checkExprEquals(
45+
"note = 'pick any array item'",
46+
"note in 'pick', 'item'",
47+
)).toBe(false);
48+
});
3849
test("genuinely different expressions are not equal", () => {
3950
expect(checkExprEquals("col >= 0", "col >= 5")).toBe(false);
4051
});
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* Column-default drift: the offline diff must emit `change-column-default` with
3+
* the right `from`/`to` payload (and omit the side that is absent), distinguish
4+
* an `expr` default from a `literal` default of the same text, and emit NOTHING
5+
* when the defaults are equal. Default drift is one of the most common real
6+
* migrations, so this pins the diff contract end to end.
7+
*/
8+
import { test, expect, describe } from "bun:test";
9+
import { diff } from "../../src/diff/index.js";
10+
import type { ColumnDescriptor, SchemaSnapshot } from "../../src/types.js";
11+
12+
function col(name: string, def?: ColumnDescriptor["default"]): ColumnDescriptor {
13+
return { name, sqlType: { kind: "text" }, nullable: true, ...(def !== undefined ? { default: def } : {}) };
14+
}
15+
function table(c: ColumnDescriptor): SchemaSnapshot {
16+
return { tables: [{ name: "t", columns: [c], indexes: [], foreignKeys: [], primaryKey: [], checks: [] }], views: [] };
17+
}
18+
function defaultChange(expected: ColumnDescriptor, actual: ColumnDescriptor) {
19+
return diff(table(expected), table(actual)).then((r) =>
20+
r.changes.find((x) => x.kind === "change-column-default"),
21+
);
22+
}
23+
24+
describe("diff — change-column-default", () => {
25+
test("literal → different literal: from/to both present", async () => {
26+
const c = await defaultChange(col("c", { kind: "literal", value: "1" }), col("c", { kind: "literal", value: "0" }));
27+
expect(c).toMatchObject({
28+
kind: "change-column-default",
29+
column: "c",
30+
from: { kind: "literal", value: "0" },
31+
to: { kind: "literal", value: "1" },
32+
});
33+
});
34+
35+
test("adding a default (actual has none): `to` present, `from` omitted", async () => {
36+
const c = await defaultChange(col("c", { kind: "literal", value: "1" }), col("c"));
37+
expect(c).toMatchObject({ to: { kind: "literal", value: "1" } });
38+
expect(c && "from" in c).toBe(false);
39+
});
40+
41+
test("removing a default (expected has none): `from` present, `to` omitted", async () => {
42+
const c = await defaultChange(col("c"), col("c", { kind: "literal", value: "1" }));
43+
expect(c).toMatchObject({ from: { kind: "literal", value: "1" } });
44+
expect(c && "to" in c).toBe(false);
45+
});
46+
47+
test("same value but different kind (expr vs literal) IS a change", async () => {
48+
const c = await defaultChange(col("c", { kind: "expr", value: "1" }), col("c", { kind: "literal", value: "1" }));
49+
expect(c).toMatchObject({
50+
from: { kind: "literal", value: "1" },
51+
to: { kind: "expr", value: "1" },
52+
});
53+
});
54+
55+
test("identical defaults → NO change emitted", async () => {
56+
const c = await defaultChange(col("c", { kind: "literal", value: "x" }), col("c", { kind: "literal", value: "x" }));
57+
expect(c).toBeUndefined();
58+
});
59+
60+
test("both sides have no default → NO change emitted", async () => {
61+
const c = await defaultChange(col("c"), col("c"));
62+
expect(c).toBeUndefined();
63+
});
64+
});

server/typescript/packages/migrate-ts/test/unit/diff-rename-heuristic.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,4 +151,50 @@ describe("rename-heuristic — table", () => {
151151
});
152152
expect(r.changes.find((c) => c.kind === "rename-column")).toBeUndefined();
153153
});
154+
155+
// Build a table whose columns are all text + nullable, so the column-set
156+
// signature (name|kind|nullable) is driven purely by the column NAMES — which
157+
// lets us dial the Jaccard overlap precisely for the threshold boundary.
158+
function tableNamed(name: string, colNames: string[]) {
159+
return {
160+
name,
161+
columns: colNames.map((n) => ({ name: n, sqlType: { kind: "text" as const }, nullable: true })),
162+
indexes: [], foreignKeys: [], primaryKey: [], checks: [],
163+
};
164+
}
165+
166+
test("Jaccard overlap == 0.8 (boundary, inclusive) → rename candidate", async () => {
167+
// drop posts {a,b,c,d} vs create articles {a,b,c,d,e}: |∩|=4, |∪|=5 → 0.8.
168+
const expected = { tables: [tableNamed("articles", ["a", "b", "c", "d", "e"])], views: [] };
169+
const actual = { tables: [tableNamed("posts", ["a", "b", "c", "d"])], views: [] };
170+
const calls: AmbiguousChange[] = [];
171+
const r = await diff({
172+
expected, actual,
173+
onAmbiguous: async (q) => { calls.push(q); return "rename"; },
174+
});
175+
expect(calls).toHaveLength(1);
176+
expect(calls[0]).toMatchObject({ kind: "possible-table-rename", columnOverlap: 0.8 });
177+
expect(r.changes.find((c) => c.kind === "rename-table")).toMatchObject({ from: "posts", to: "articles" });
178+
});
179+
180+
test("Jaccard overlap just below 0.8 → NOT a candidate (no callback)", async () => {
181+
// drop posts {a,b,c} vs create articles {a,b,c,d}: |∩|=3, |∪|=4 → 0.75 < 0.8.
182+
const expected = { tables: [tableNamed("articles", ["a", "b", "c", "d"])], views: [] };
183+
const actual = { tables: [tableNamed("posts", ["a", "b", "c"])], views: [] };
184+
const calls: AmbiguousChange[] = [];
185+
await diff({
186+
expected, actual, allow: { dropTable: true },
187+
onAmbiguous: async (q) => { calls.push(q); return "rename"; },
188+
});
189+
expect(calls).toHaveLength(0);
190+
});
191+
192+
test("table candidate with 'abort' resolution throws", async () => {
193+
const expected = { tables: [table("articles")], views: [] };
194+
const actual = { tables: [table("posts")], views: [] };
195+
await expect(diff({
196+
expected, actual,
197+
onAmbiguous: async () => "abort",
198+
})).rejects.toThrow(/possible rename posts articles/);
199+
});
154200
});

server/typescript/packages/migrate-ts/test/unit/diff.test.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ describe("diff — per-table column-level", () => {
5858
const tableA = { name: "users", columns: [col("count", "text")], indexes: [], foreignKeys: [], primaryKey: [], checks: [] };
5959
const r = await diff(snapshot([tableE]), snapshot([tableA]));
6060
const c = r.changes.find((x) => x.kind === "change-column-type");
61-
expect(c).toBeDefined();
61+
expect(c).toMatchObject({ kind: "change-column-type", table: "users", column: "count" });
6262
});
6363

6464
test("nullable mismatch → change-column-nullable", async () => {
@@ -74,7 +74,9 @@ describe("diff — per-table column-level", () => {
7474
const tableE = { name: "users", columns: [{ ...col("flag", "boolean"), default: { kind: "literal" as const, value: "true" } }], indexes: [], foreignKeys: [], primaryKey: [], checks: [] };
7575
const tableA = { name: "users", columns: [col("flag", "boolean")], indexes: [], foreignKeys: [], primaryKey: [], checks: [] };
7676
const r = await diff(snapshot([tableE]), snapshot([tableA]));
77-
expect(r.changes.find((x) => x.kind === "change-column-default")).toBeDefined();
77+
expect(r.changes.find((x) => x.kind === "change-column-default")).toMatchObject({
78+
kind: "change-column-default", table: "users", column: "flag", to: { kind: "literal", value: "true" },
79+
});
7880
});
7981
});
8082

@@ -83,14 +85,18 @@ describe("diff — per-table index/FK", () => {
8385
const tableE = { name: "users", columns: [col("id", "integer"), col("email")], indexes: [{ name: "users_email_idx", columns: ["email"], unique: true }], foreignKeys: [], primaryKey: ["id"], checks: [] };
8486
const tableA = { name: "users", columns: [col("id", "integer"), col("email")], indexes: [], foreignKeys: [], primaryKey: ["id"], checks: [] };
8587
const r = await diff(snapshot([tableE]), snapshot([tableA]));
86-
expect(r.changes.find((x) => x.kind === "add-index")).toBeDefined();
88+
expect(r.changes.find((x) => x.kind === "add-index")).toMatchObject({
89+
kind: "add-index", table: "users", index: { name: "users_email_idx", columns: ["email"], unique: true },
90+
});
8791
});
8892

8993
test("actual FK not in expected → drop-fk", async () => {
9094
const tableE = { name: "weeks", columns: [col("id", "integer"), col("program_id", "integer")], indexes: [], foreignKeys: [], primaryKey: ["id"], checks: [] };
9195
const tableA = { name: "weeks", columns: [col("id", "integer"), col("program_id", "integer")], indexes: [], foreignKeys: [{ name: "weeks_program_id_fk", columns: ["program_id"], refTable: "programs", refColumns: ["id"] }], primaryKey: ["id"], checks: [] };
9296
const r = await diff(snapshot([tableE]), snapshot([tableA]));
93-
expect(r.changes.find((x) => x.kind === "drop-fk")).toBeDefined();
97+
expect(r.changes.find((x) => x.kind === "drop-fk")).toMatchObject({
98+
kind: "drop-fk", table: "weeks", fk: "weeks_program_id_fk",
99+
});
94100
});
95101
});
96102

@@ -117,7 +123,7 @@ describe("diff — view-body drift", () => {
117123
]);
118124
const r = await diff(expected, actual);
119125
const viewChange = r.changes.find((c) => c.kind === "replace-view");
120-
expect(viewChange).toBeDefined();
126+
expect(viewChange).toMatchObject({ kind: "replace-view", view: { name: "order_summary" } });
121127
// No spurious create/drop pair when the view exists on both sides.
122128
expect(r.changes.find((c) => c.kind === "create-view")).toBeUndefined();
123129
expect(r.changes.find((c) => c.kind === "drop-view")).toBeUndefined();
@@ -126,6 +132,8 @@ describe("diff — view-body drift", () => {
126132
test("expected view absent from actual → create-view (unchanged)", async () => {
127133
const expected = withViews([{ name: "order_summary", sql: "SELECT id FROM orders" }]);
128134
const r = await diff(expected, { tables: [], views: [] });
129-
expect(r.changes.find((c) => c.kind === "create-view")).toBeDefined();
135+
expect(r.changes.find((c) => c.kind === "create-view")).toMatchObject({
136+
kind: "create-view", view: { name: "order_summary" },
137+
});
130138
});
131139
});
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* SQLite emits native ALTER for column rename/drop only on new-enough engines,
3+
* and falls back to recreate-and-copy on older ones. The version boundaries are
4+
* load-bearing — pick the wrong DDL and the migration fails on the target
5+
* engine. These tests pin the exact thresholds:
6+
* - native RENAME COLUMN: SQLite >= 3.25.0
7+
* - native DROP COLUMN: SQLite >= 3.35.0
8+
* and that an unparseable/absent version is treated as modern.
9+
*/
10+
import { test, expect, describe } from "bun:test";
11+
import { emit } from "../../src/emit/index.js";
12+
import type { Change } from "../../src/types.js";
13+
import type { SchemaSnapshot } from "../../src/types.js";
14+
15+
const TABLE: SchemaSnapshot = {
16+
tables: [{
17+
name: "t",
18+
columns: [{ name: "a", sqlType: { kind: "text" }, nullable: true }],
19+
indexes: [],
20+
foreignKeys: [],
21+
primaryKey: [],
22+
checks: [],
23+
}],
24+
views: [],
25+
};
26+
27+
function emitSqlite(changes: Change[], sqliteVersion?: string) {
28+
return emit(changes, {
29+
dialect: "sqlite",
30+
expectedSchema: TABLE,
31+
// Omit actualMeta entirely when no version (exactOptionalPropertyTypes
32+
// forbids assigning `undefined` to the optional property).
33+
...(sqliteVersion !== undefined ? { actualMeta: { sqliteVersion } } : {}),
34+
});
35+
}
36+
37+
const renameChange: Change[] = [{ kind: "rename-column", status: { state: "allowed" }, table: "t", from: "a", to: "b" }];
38+
const dropChange: Change[] = [{ kind: "drop-column", status: { state: "allowed" }, table: "t", column: "a" }];
39+
40+
describe("SQLite rename-column version gating (>= 3.25.0 native)", () => {
41+
test("3.25.0 → native ALTER … RENAME COLUMN (no recreate)", () => {
42+
const r = emitSqlite(renameChange, "3.25.0");
43+
expect(r.up).toContain("RENAME COLUMN");
44+
expect([...r.recreatedTables]).toEqual([]);
45+
});
46+
47+
test("3.24.0 (one patch below) → recreate-and-copy", () => {
48+
const r = emitSqlite(renameChange, "3.24.0");
49+
expect(r.up).not.toMatch(/RENAME COLUMN/);
50+
expect(r.up).toContain("CREATE TABLE");
51+
expect([...r.recreatedTables]).toEqual(["t"]);
52+
});
53+
});
54+
55+
describe("SQLite drop-column version gating (>= 3.35.0 native)", () => {
56+
test("3.35.0 → native ALTER … DROP COLUMN (no recreate)", () => {
57+
const r = emitSqlite(dropChange, "3.35.0");
58+
expect(r.up).toMatch(/DROP COLUMN/i);
59+
expect([...r.recreatedTables]).toEqual([]);
60+
});
61+
62+
test("3.34.0 (one minor below) → recreate-and-copy", () => {
63+
const r = emitSqlite(dropChange, "3.34.0");
64+
expect(r.up).not.toMatch(/DROP COLUMN/i);
65+
expect(r.up).toContain("CREATE TABLE");
66+
expect([...r.recreatedTables]).toEqual(["t"]);
67+
});
68+
});
69+
70+
describe("SQLite version parsing fallback", () => {
71+
test("an unparseable version string is treated as modern (native DDL)", () => {
72+
const r = emitSqlite(dropChange, "garbage");
73+
expect(r.up).toMatch(/DROP COLUMN/i);
74+
expect([...r.recreatedTables]).toEqual([]);
75+
});
76+
77+
test("an absent version (no actualMeta) is treated as modern (native DDL)", () => {
78+
const r = emitSqlite(dropChange, undefined);
79+
expect(r.up).toMatch(/DROP COLUMN/i);
80+
expect([...r.recreatedTables]).toEqual([]);
81+
});
82+
});
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/**
2+
* Pure-unit coverage for the Postgres introspection helpers. These are exported
3+
* precisely so they can be tested WITHOUT a live database — the pg-mem-backed
4+
* tests can't exercise them (pg-mem returns null for column_default and type
5+
* metadata), and the real-Postgres round-trip tests are gated on
6+
* MIGRATE_TS_PG_URL. This file pins the `--from-db` type/default mapping that
7+
* migration correctness rides on.
8+
*/
9+
import { test, expect, describe } from "bun:test";
10+
import { pgTypeToSqlType, parsePgDefault } from "../../src/introspect/postgres.js";
11+
12+
describe("pgTypeToSqlType", () => {
13+
test("integer family maps to the modeled bit width (smallint folds into 32)", () => {
14+
expect(pgTypeToSqlType("bigint")).toEqual({ kind: "integer", bits: 64 });
15+
expect(pgTypeToSqlType("int8")).toEqual({ kind: "integer", bits: 64 });
16+
expect(pgTypeToSqlType("bigserial")).toEqual({ kind: "integer", bits: 64 });
17+
expect(pgTypeToSqlType("integer")).toEqual({ kind: "integer", bits: 32 });
18+
expect(pgTypeToSqlType("serial")).toEqual({ kind: "integer", bits: 32 });
19+
expect(pgTypeToSqlType("smallint")).toEqual({ kind: "integer", bits: 32 });
20+
});
21+
22+
test("varchar carries maxLength inline or from the separate column", () => {
23+
expect(pgTypeToSqlType("character varying(255)")).toEqual({ kind: "text", maxLength: 255 });
24+
expect(pgTypeToSqlType("varchar(64)")).toEqual({ kind: "text", maxLength: 64 });
25+
// bare varchar + separate maxLength column
26+
expect(pgTypeToSqlType("character varying", 100)).toEqual({ kind: "text", maxLength: 100 });
27+
// bare varchar with no length info → plain text
28+
expect(pgTypeToSqlType("character varying")).toEqual({ kind: "text" });
29+
expect(pgTypeToSqlType("text")).toEqual({ kind: "text" });
30+
});
31+
32+
test("numeric carries precision/scale when present", () => {
33+
expect(pgTypeToSqlType("numeric(10,2)")).toEqual({ kind: "numeric", precision: 10, scale: 2 });
34+
expect(pgTypeToSqlType("numeric(8)")).toEqual({ kind: "numeric", precision: 8 });
35+
expect(pgTypeToSqlType("numeric")).toEqual({ kind: "numeric" });
36+
expect(pgTypeToSqlType("decimal(12,4)")).toEqual({ kind: "numeric", precision: 12, scale: 4 });
37+
});
38+
39+
test("floating point distinguishes single vs double precision", () => {
40+
expect(pgTypeToSqlType("real")).toEqual({ kind: "real4" });
41+
expect(pgTypeToSqlType("float4")).toEqual({ kind: "real4" });
42+
expect(pgTypeToSqlType("double precision")).toEqual({ kind: "real" });
43+
expect(pgTypeToSqlType("float8")).toEqual({ kind: "real" });
44+
});
45+
46+
test("temporal types carry timezone awareness", () => {
47+
expect(pgTypeToSqlType("date")).toEqual({ kind: "date" });
48+
expect(pgTypeToSqlType("timestamp without time zone")).toEqual({ kind: "timestamp", withTimezone: false });
49+
expect(pgTypeToSqlType("timestamptz")).toEqual({ kind: "timestamp", withTimezone: true });
50+
expect(pgTypeToSqlType("timestamp with time zone")).toEqual({ kind: "timestamp", withTimezone: true });
51+
});
52+
53+
test("boolean, json, binary, uuid", () => {
54+
expect(pgTypeToSqlType("boolean")).toEqual({ kind: "boolean" });
55+
expect(pgTypeToSqlType("jsonb")).toEqual({ kind: "json" });
56+
expect(pgTypeToSqlType("json")).toEqual({ kind: "json" });
57+
expect(pgTypeToSqlType("bytea")).toEqual({ kind: "blob" });
58+
expect(pgTypeToSqlType("uuid")).toEqual({ kind: "uuid" });
59+
});
60+
61+
test("unknown / user-defined types fall back to text (don't blow up)", () => {
62+
expect(pgTypeToSqlType("citext")).toEqual({ kind: "text" });
63+
expect(pgTypeToSqlType("ltree")).toEqual({ kind: "text" });
64+
expect(pgTypeToSqlType("my_custom_enum")).toEqual({ kind: "text" });
65+
});
66+
67+
test("type matching is case-insensitive and trims", () => {
68+
expect(pgTypeToSqlType(" BIGINT ")).toEqual({ kind: "integer", bits: 64 });
69+
expect(pgTypeToSqlType("TIMESTAMPTZ")).toEqual({ kind: "timestamp", withTimezone: true });
70+
});
71+
});
72+
73+
describe("parsePgDefault", () => {
74+
test("absent default → undefined", () => {
75+
expect(parsePgDefault(null)).toBeUndefined();
76+
expect(parsePgDefault(undefined)).toBeUndefined();
77+
expect(parsePgDefault("")).toBeUndefined();
78+
});
79+
80+
test("function/keyword expressions are kind=expr (verbatim)", () => {
81+
expect(parsePgDefault("now()")).toEqual({ kind: "expr", value: "now()" });
82+
expect(parsePgDefault("CURRENT_TIMESTAMP")).toEqual({ kind: "expr", value: "CURRENT_TIMESTAMP" });
83+
expect(parsePgDefault("CURRENT_DATE")).toEqual({ kind: "expr", value: "CURRENT_DATE" });
84+
expect(parsePgDefault("nextval('users_id_seq'::regclass)")).toEqual({
85+
kind: "expr",
86+
value: "nextval('users_id_seq'::regclass)",
87+
});
88+
});
89+
90+
test("quoted literals strip the ::type cast and the surrounding quotes", () => {
91+
expect(parsePgDefault("'hi'::text")).toEqual({ kind: "literal", value: "hi" });
92+
expect(parsePgDefault("'true'::boolean")).toEqual({ kind: "literal", value: "true" });
93+
expect(parsePgDefault("'42'::integer")).toEqual({ kind: "literal", value: "42" });
94+
// no trailing cast
95+
expect(parsePgDefault("'plain'")).toEqual({ kind: "literal", value: "plain" });
96+
});
97+
98+
test("a bare non-quoted value with a :: cast is a complex expression", () => {
99+
expect(parsePgDefault("NULL::text")).toEqual({ kind: "expr", value: "NULL::text" });
100+
expect(parsePgDefault("ARRAY[]::text[]")).toEqual({ kind: "expr", value: "ARRAY[]::text[]" });
101+
});
102+
103+
test("a bare literal with no quotes and no cast is kind=literal", () => {
104+
expect(parsePgDefault("42")).toEqual({ kind: "literal", value: "42" });
105+
});
106+
});

0 commit comments

Comments
 (0)