ReviewOS
Reload

also looking at this

stacks/bun-query-builder

fix(migrate)!: run the directories executeMigration was given

#1138
Merged glennmichael123 wants to merge fix/execute-migration-dirs into main
7 files +348 -49
packages/bun-query-builder/test/migrate-multiple-dirs.test.tsadded+190-0
@@ -0,0 +1,190 @@
1/**
2 * Running one migration corpus that lives in more than one directory.
3 *
4 * `executeMigration` declared a `dir` argument and then threw it away, always
5 * enumerating `config.migrationDir` — so a caller could not point it anywhere
6 * else, and a caller pointing it at the wrong place never found out. Honouring
7 * the argument is the small half. The half that matters is a list: an
8 * application's own migrations plus the ones each installed package ships,
9 * where a package's tables carry foreign keys into the application's and never
10 * the reverse.
11 *
12 * Running each directory in turn does not work, because the file name is the
13 * ordinal and per-directory sorting loses the cross-directory order. These
14 * tests pin the global ordering, and the two ways a multi-directory run can go
15 * quietly wrong: a name that repeats across directories (the ledger keys on the
16 * bare name, so one file would be recorded and the other silently skipped) and
17 * a directory that is not there.
18 */
19import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'bun:test'
20import { Database } from 'bun:sqlite'
21import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
22import { tmpdir } from 'node:os'
23import { join } from 'node:path'
24import process from 'node:process'
25import { executeMigration } from '../src/actions/migrate'
26import { config, setConfig } from '../src/config'
27
28let workspace: string
29let appDir: string
30let packageDir: string
31let dbFile: string
32let originalCwd: string
33let prevDatabase: any
34let prevDialect: any
35let dbCounter = 0
36
37/** The application's own corpus, at the configured `database/migrations`. */
38const USERS = '0000000001-create-users.sql'
39const USERS_SQL = 'CREATE TABLE "users" ("id" INTEGER PRIMARY KEY, "email" TEXT);\n'
40
41/**
42 * The application's, again — but ordered AFTER the package's file, and
43 * depending on it. A run that sorts each directory separately reaches this one
44 * while "posts" does not yet exist, and `ALTER TABLE` on a missing table fails
45 * on every dialect including SQLite, which tolerates a dangling REFERENCES.
46 */
47const AUTHORS = '0000000003-add-author-to-posts.sql'
48const AUTHORS_SQL = 'ALTER TABLE "posts" ADD COLUMN "author_id" INTEGER;\n'
49
50/** The package's, ordered between the two application files. */
51const POSTS = '0000000002-create-posts.sql'
52const POSTS_SQL = 'CREATE TABLE "posts" ("id" INTEGER PRIMARY KEY, "user_id" INTEGER REFERENCES "users"("id"));\n'
53
54/** Point the builder at a database file no migration has ever run against. */
55function useFreshDatabase(): string {
56 dbCounter += 1
57 dbFile = join(workspace, 'database', `app-${dbCounter}.sqlite`)
58 setConfig({ dialect: 'sqlite', database: { ...prevDatabase, database: dbFile } })
59 return dbFile
60}
61
62/** Reads the database as it stands, treating "never created" as "nothing there". */
63function read<T>(query: string): T[] {
64 if (!existsSync(dbFile))
65 return []
66
67 const db = new Database(dbFile, { readonly: true })
68 try { return db.query(query).all() as T[] }
69 finally { db.close() }
70}
71
72function recorded(): string[] {
73 return read<{ migration: string }>('SELECT migration FROM migrations ORDER BY id').map(row => row.migration)
74}
75
76function columnsOf(table: string): string[] {
77 return read<{ name: string }>(`PRAGMA table_info("${table}")`).map(column => column.name)
78}
79
80beforeAll(() => {
81 originalCwd = process.cwd()
82 workspace = realpathSync(mkdtempSync(join(tmpdir(), 'qb-multi-dir-')))
83 appDir = join(workspace, 'database', 'migrations')
84 packageDir = join(workspace, 'node_modules', 'loghq', 'database', 'migrations')
85 mkdirSync(appDir, { recursive: true })
86 mkdirSync(packageDir, { recursive: true })
87 writeFileSync(join(workspace, 'package.json'), '{"name":"multi-dir-fixture"}')
88 process.chdir(workspace)
89
90 prevDatabase = config.database
91 prevDialect = config.dialect
92})
93
94afterAll(() => {
95 setConfig({ dialect: prevDialect, database: prevDatabase })
96 process.chdir(originalCwd)
97 rmSync(workspace, { recursive: true, force: true })
98})
99
100beforeEach(() => {
101 rmSync(appDir, { recursive: true, force: true })
102 rmSync(packageDir, { recursive: true, force: true })
103 mkdirSync(appDir, { recursive: true })
104 mkdirSync(packageDir, { recursive: true })
105 useFreshDatabase()
106})
107
108describe('executeMigration across several directories', () => {
109 beforeEach(() => {
110 writeFileSync(join(appDir, USERS), USERS_SQL)
111 writeFileSync(join(appDir, AUTHORS), AUTHORS_SQL)
112 writeFileSync(join(packageDir, POSTS), POSTS_SQL)
113 })
114
115 it('orders every file by name across all of them, not one directory at a time', async () => {
116 await executeMigration([appDir, packageDir])
117
118 expect(recorded()).toEqual([USERS, POSTS, AUTHORS])
119 expect(columnsOf('posts')).toContain('author_id')
120 })
121
122 it('orders the same way whichever order the directories are given in', async () => {
123 await executeMigration([packageDir, appDir])
124
125 expect(recorded()).toEqual([USERS, POSTS, AUTHORS])
126 })
127
128 it('records the bare file name, whichever directory the file came from', async () => {
129 await executeMigration([appDir, packageDir])
130
131 expect(recorded()).not.toContain(join(packageDir, POSTS))
132 })
133
134 it('runs nothing on a second pass', async () => {
135 await executeMigration([appDir, packageDir])
136 await executeMigration([appDir, packageDir])
137
138 expect(recorded()).toEqual([USERS, POSTS, AUTHORS])
139 })
140
141 it('refuses a directory that is not there rather than reporting an empty corpus', async () => {
142 const missing = join(workspace, 'node_modules', 'not-installed', 'database', 'migrations')
143
144 await expect(executeMigration([appDir, missing])).rejects.toThrow(/not found/i)
145 expect(recorded()).toEqual([])
146 })
147
148 it('refuses a file name that repeats across directories', async () => {
149 // The ledger keys on the bare name, so the second file would look already
150 // executed and be skipped — a migration that never ran, on a run that
151 // reported success.
152 writeFileSync(join(packageDir, USERS), 'CREATE TABLE "loghq_users" ("id" INTEGER PRIMARY KEY);\n')
153
154 await expect(executeMigration([appDir, packageDir])).rejects.toThrow(new RegExp(USERS))
155 expect(recorded()).toEqual([])
156 })
157
158 it('refuses an empty list rather than guessing what it meant', async () => {
159 await expect(executeMigration([])).rejects.toThrow(/empty list/i)
160 })
161})
162
163describe('executeMigration with a single directory', () => {
164 it('runs the directory it was given', async () => {
165 writeFileSync(join(packageDir, POSTS), 'CREATE TABLE "posts" ("id" INTEGER PRIMARY KEY);\n')
166
167 await executeMigration(packageDir)
168
169 expect(recorded()).toEqual([POSTS])
170 })
171
172 it('resolves a relative directory against the workspace root', async () => {
173 writeFileSync(join(packageDir, POSTS), 'CREATE TABLE "posts" ("id" INTEGER PRIMARY KEY);\n')
174
175 await executeMigration('node_modules/loghq/database/migrations')
176
177 expect(recorded()).toEqual([POSTS])
178 })
179})
180
181describe('executeMigration with no argument', () => {
182 it('still runs the configured migrationDir, so existing callers are unchanged', async () => {
183 writeFileSync(join(appDir, USERS), USERS_SQL)
184 writeFileSync(join(packageDir, POSTS), POSTS_SQL)
185
186 await executeMigration()
187
188 expect(recorded()).toEqual([USERS])
189 })
190})