|
| 1 | +/** |
| 2 | + * Run with: bun test scripts/check-migrations-safety.test.ts |
| 3 | + * (Root scripts are bun-native and not part of the turbo/vitest workspaces.) |
| 4 | + */ |
| 5 | +import { describe, expect, test } from 'bun:test' |
| 6 | +import { lintSql } from './check-migrations-safety.ts' |
| 7 | + |
| 8 | +const rules = (sql: string) => lintSql(sql).map((f) => `${f.tier}:${f.rule}`) |
| 9 | + |
| 10 | +describe('additive / safe', () => { |
| 11 | + test('nullable add column passes', () => { |
| 12 | + expect(lintSql('ALTER TABLE "webhook" ADD COLUMN "provider_config" json;')).toEqual([]) |
| 13 | + }) |
| 14 | + |
| 15 | + test('NOT NULL with DEFAULT passes', () => { |
| 16 | + expect(lintSql('ALTER TABLE "user" ADD COLUMN "flag" boolean DEFAULT false NOT NULL;')).toEqual( |
| 17 | + [] |
| 18 | + ) |
| 19 | + }) |
| 20 | + |
| 21 | + test('CREATE TABLE plus index and FK on that new table passes', () => { |
| 22 | + const sql = `CREATE TABLE "kb" ("id" text PRIMARY KEY NOT NULL, "user_id" text NOT NULL); |
| 23 | +--> statement-breakpoint |
| 24 | +CREATE INDEX "kb_user_id_idx" ON "kb" USING btree ("user_id"); |
| 25 | +--> statement-breakpoint |
| 26 | +ALTER TABLE "kb" ADD CONSTRAINT "kb_user_fk" FOREIGN KEY ("user_id") REFERENCES "user"("id");` |
| 27 | + expect(lintSql(sql)).toEqual([]) |
| 28 | + }) |
| 29 | + |
| 30 | + test('CONCURRENTLY index after a COMMIT breakpoint passes', () => { |
| 31 | + const sql = `COMMIT; |
| 32 | +--> statement-breakpoint |
| 33 | +SET lock_timeout = 0; |
| 34 | +--> statement-breakpoint |
| 35 | +CREATE INDEX CONCURRENTLY IF NOT EXISTS "idx_x" ON "embedding" ("kb_id");` |
| 36 | + expect(lintSql(sql)).toEqual([]) |
| 37 | + }) |
| 38 | +}) |
| 39 | + |
| 40 | +describe('hard errors', () => { |
| 41 | + test('ADD COLUMN NOT NULL without default', () => { |
| 42 | + expect(rules('ALTER TABLE "user" ADD COLUMN "email" text NOT NULL;')).toEqual([ |
| 43 | + 'error:add-not-null-no-default', |
| 44 | + ]) |
| 45 | + }) |
| 46 | + |
| 47 | + test('RENAME column', () => { |
| 48 | + expect(rules('ALTER TABLE "marketplace" RENAME COLUMN "executions" TO "views";')).toEqual([ |
| 49 | + 'error:rename', |
| 50 | + ]) |
| 51 | + }) |
| 52 | + |
| 53 | + test('CREATE INDEX on existing table without CONCURRENTLY', () => { |
| 54 | + expect(rules('CREATE INDEX "idx_y" ON "embedding" ("kb_id");')).toEqual([ |
| 55 | + 'error:index-not-concurrent', |
| 56 | + ]) |
| 57 | + }) |
| 58 | + |
| 59 | + test('CONCURRENTLY index without IF NOT EXISTS', () => { |
| 60 | + const sql = `COMMIT; |
| 61 | +--> statement-breakpoint |
| 62 | +CREATE INDEX CONCURRENTLY "idx_z" ON "embedding" ("kb_id");` |
| 63 | + expect(rules(sql)).toEqual(['error:concurrent-index-not-idempotent']) |
| 64 | + }) |
| 65 | + |
| 66 | + test('CONCURRENTLY index without a preceding COMMIT', () => { |
| 67 | + expect( |
| 68 | + rules('CREATE INDEX CONCURRENTLY IF NOT EXISTS "idx_z" ON "embedding" ("kb_id");') |
| 69 | + ).toEqual(['error:concurrent-index-no-commit']) |
| 70 | + }) |
| 71 | + |
| 72 | + test('ADD FOREIGN KEY on existing table without NOT VALID', () => { |
| 73 | + expect( |
| 74 | + rules( |
| 75 | + 'ALTER TABLE "session" ADD CONSTRAINT "s_fk" FOREIGN KEY ("uid") REFERENCES "user"("id");' |
| 76 | + ) |
| 77 | + ).toEqual(['error:constraint-not-valid']) |
| 78 | + }) |
| 79 | +}) |
| 80 | + |
| 81 | +describe('annotate tier', () => { |
| 82 | + const drop = 'ALTER TABLE "webhook" DROP COLUMN "secret";' |
| 83 | + |
| 84 | + test('DROP COLUMN unannotated fails', () => { |
| 85 | + expect(rules(drop)).toEqual(['error:drop-column']) |
| 86 | + }) |
| 87 | + |
| 88 | + test('DROP COLUMN annotated passes', () => { |
| 89 | + const sql = `-- migration-safe: secret read removed in v0.6.1 (#1234), shipped two deploys ago\n${drop}` |
| 90 | + expect(lintSql(sql)).toEqual([]) |
| 91 | + }) |
| 92 | + |
| 93 | + test('annotation tolerates an intervening statement-breakpoint line', () => { |
| 94 | + const sql = `ALTER TABLE "webhook" ADD COLUMN "provider_config" json; |
| 95 | +--> statement-breakpoint |
| 96 | +-- migration-safe: secret read removed in v0.6.1 (#1234) |
| 97 | +${drop}` |
| 98 | + expect(lintSql(sql)).toEqual([]) |
| 99 | + }) |
| 100 | + |
| 101 | + test('dangling annotation with empty reason fails', () => { |
| 102 | + const sql = `-- migration-safe:\n${drop}` |
| 103 | + const found = lintSql(sql) |
| 104 | + expect(found).toHaveLength(1) |
| 105 | + expect(found[0].tier).toBe('error') |
| 106 | + expect(found[0].message).toContain('no reason') |
| 107 | + }) |
| 108 | + |
| 109 | + test('annotation on the wrong statement does not bleed', () => { |
| 110 | + const sql = `-- migration-safe: removing secret |
| 111 | +ALTER TABLE "webhook" ADD COLUMN "x" json; |
| 112 | +--> statement-breakpoint |
| 113 | +${drop}` |
| 114 | + expect(rules(sql)).toEqual(['error:drop-column']) |
| 115 | + }) |
| 116 | + |
| 117 | + test('type change and DROP TABLE are annotate-tier', () => { |
| 118 | + expect( |
| 119 | + rules( |
| 120 | + 'ALTER TABLE "user_table_rows" ALTER COLUMN "order_key" SET DATA TYPE text COLLATE "C";' |
| 121 | + ) |
| 122 | + ).toEqual(['error:alter-type']) |
| 123 | + expect(rules('DROP TABLE "marketplace_execution" CASCADE;')).toEqual(['error:drop-table']) |
| 124 | + }) |
| 125 | +}) |
| 126 | + |
| 127 | +describe('warnings (non-blocking)', () => { |
| 128 | + test('UPDATE backfill warns but does not error', () => { |
| 129 | + const found = lintSql('UPDATE "user_table_definitions" SET "schema" = \'{}\' WHERE id = \'1\';') |
| 130 | + expect(found.map((f) => f.tier)).toEqual(['warn']) |
| 131 | + }) |
| 132 | + |
| 133 | + test('UPDATE without WHERE flags the whole-table note', () => { |
| 134 | + const found = lintSql('UPDATE "user" SET "active" = true;') |
| 135 | + expect(found[0].tier).toBe('warn') |
| 136 | + expect(found[0].message).toContain('no WHERE') |
| 137 | + }) |
| 138 | +}) |
| 139 | + |
| 140 | +describe('review fixes', () => { |
| 141 | + test('RENAME CONSTRAINT is metadata-only — not flagged', () => { |
| 142 | + expect( |
| 143 | + lintSql('ALTER TABLE "permission_group" RENAME CONSTRAINT "old_fk" TO "new_fk";') |
| 144 | + ).toEqual([]) |
| 145 | + }) |
| 146 | + |
| 147 | + test('ALTER INDEX ... RENAME is metadata-only — not flagged', () => { |
| 148 | + expect(lintSql('ALTER INDEX "old_idx" RENAME TO "new_idx";')).toEqual([]) |
| 149 | + }) |
| 150 | + |
| 151 | + test('table RENAME TO is still a hard error', () => { |
| 152 | + expect(rules('ALTER TABLE "marketplace" RENAME TO "listings";')).toEqual(['error:rename']) |
| 153 | + }) |
| 154 | + |
| 155 | + test('plain DROP INDEX is a hard error (ACCESS EXCLUSIVE lock)', () => { |
| 156 | + expect(rules('DROP INDEX "permission_group_workspace_name_unique";')).toEqual([ |
| 157 | + 'error:drop-index-not-concurrent', |
| 158 | + ]) |
| 159 | + }) |
| 160 | + |
| 161 | + test('DROP INDEX CONCURRENTLY after a COMMIT passes clean', () => { |
| 162 | + const sql = `COMMIT; |
| 163 | +--> statement-breakpoint |
| 164 | +DROP INDEX CONCURRENTLY IF EXISTS "stale_idx";` |
| 165 | + expect(lintSql(sql)).toEqual([]) |
| 166 | + }) |
| 167 | + |
| 168 | + test('DROP INDEX CONCURRENTLY without IF EXISTS is not idempotent', () => { |
| 169 | + const sql = `COMMIT; |
| 170 | +--> statement-breakpoint |
| 171 | +DROP INDEX CONCURRENTLY "stale_idx";` |
| 172 | + expect(rules(sql)).toEqual(['error:concurrent-drop-index-not-idempotent']) |
| 173 | + }) |
| 174 | + |
| 175 | + test('DROP INDEX CONCURRENTLY without a preceding COMMIT errors', () => { |
| 176 | + expect(rules('DROP INDEX CONCURRENTLY IF EXISTS "stale_idx";')).toEqual([ |
| 177 | + 'error:concurrent-drop-index-no-commit', |
| 178 | + ]) |
| 179 | + }) |
| 180 | + |
| 181 | + test('alter-type does not match TYPE inside a string default', () => { |
| 182 | + expect(lintSql(`ALTER TABLE "x" ALTER COLUMN "y" SET DEFAULT 'change TYPE later';`)).toEqual([]) |
| 183 | + }) |
| 184 | +}) |
| 185 | + |
| 186 | +describe('parser robustness', () => { |
| 187 | + test('semicolon inside a string literal does not split', () => { |
| 188 | + expect(lintSql(`ALTER TABLE "x" ADD COLUMN "y" text DEFAULT 'a;b' NOT NULL;`)).toEqual([]) |
| 189 | + }) |
| 190 | + |
| 191 | + test('dollar-quoted DO block is one statement; FK on a new table is suppressed', () => { |
| 192 | + const sql = `CREATE TABLE "jobs" ("id" text PRIMARY KEY NOT NULL, "wid" text NOT NULL); |
| 193 | +--> statement-breakpoint |
| 194 | +DO $$ BEGIN |
| 195 | + ALTER TABLE "jobs" ADD CONSTRAINT "jobs_fk" FOREIGN KEY ("wid") REFERENCES "workspace"("id"); |
| 196 | +EXCEPTION WHEN duplicate_object THEN null; |
| 197 | +END $$;` |
| 198 | + expect(lintSql(sql)).toEqual([]) |
| 199 | + }) |
| 200 | +}) |
0 commit comments