Skip to content

Commit 6238380

Browse files
committed
test(migration): pin the 0107 review_check_mode backfill against a real pre-migration row (#2852)
The existing reviewCheckMode tests only exercise the application layer through createTestEnv(), which applies every migration (including 0107) up front against an empty table -- so the migration's own UPDATE backfill never ran against a real pre-existing row and could drift silently. Mirrors migration-0102-linked-issue-gate-mode.test.ts's approach: replay every migration before 0107 into a fresh in-memory DB, insert a raw gate_check_mode='enabled' row exactly as a pre-existing installed repo would have it, then apply migration 0107 for real and assert the backfill. Also covers the 'off' and unrecognized-legacy-value cases failing closed to the new column's 'disabled' default, and a multi-row backfill in one run.
1 parent 702b282 commit 6238380

1 file changed

Lines changed: 81 additions & 0 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { readFileSync, readdirSync } from "node:fs";
2+
import { DatabaseSync } from "node:sqlite";
3+
import { describe, expect, it } from "vitest";
4+
5+
const MIGRATION_FILE = "0107_repository_review_check_mode.sql";
6+
7+
// Replays every migrations/*.sql file BEFORE 0107 into a fresh in-memory DB (mirrors
8+
// migration-0102-linked-issue-gate-mode.test.ts's own approach), so the table shape this test inserts into is
9+
// exactly what migration 0107 itself was written against. The TestD1Database helper (test/helpers/d1.ts) can't
10+
// be reused here: it concatenates and applies EVERY migration (including 0107) up front, so the new
11+
// review_check_mode column would already be backfilled by the time a test could insert a pre-migration
12+
// gate_check_mode='enabled' row -- there would be nothing left for 0107's UPDATE to actually backfill.
13+
function applyMigrationsBefore(cutoffFile: string): DatabaseSync {
14+
const db = new DatabaseSync(":memory:");
15+
const files = readdirSync("migrations")
16+
.filter((file) => file.endsWith(".sql") && file < cutoffFile)
17+
.sort();
18+
for (const file of files) db.exec(readFileSync(`migrations/${file}`, "utf8"));
19+
return db;
20+
}
21+
22+
function applyMigration(db: DatabaseSync, file: string): void {
23+
db.exec(readFileSync(`migrations/${file}`, "utf8"));
24+
}
25+
26+
function insertRepositorySettingsRow(db: DatabaseSync, repoFullName: string, gateCheckMode: string): void {
27+
db.prepare("INSERT INTO repository_settings (repo_full_name, gate_check_mode) VALUES (?, ?)").run(repoFullName, gateCheckMode);
28+
}
29+
30+
function readReviewCheckMode(db: DatabaseSync, repoFullName: string): string {
31+
const row = db.prepare("SELECT review_check_mode FROM repository_settings WHERE repo_full_name = ?").get(repoFullName) as
32+
| { review_check_mode: string }
33+
| undefined;
34+
if (!row) throw new Error(`no repository_settings row for ${repoFullName}`);
35+
return row.review_check_mode;
36+
}
37+
38+
describe("migration 0107: review_check_mode backfill (#2852)", () => {
39+
it("backfills a pre-existing gate_check_mode='enabled' row to review_check_mode='required', via the real migration", () => {
40+
const db = applyMigrationsBefore(MIGRATION_FILE);
41+
insertRepositorySettingsRow(db, "acme/legacy-enabled", "enabled");
42+
43+
applyMigration(db, MIGRATION_FILE);
44+
45+
expect(readReviewCheckMode(db, "acme/legacy-enabled")).toBe("required");
46+
});
47+
48+
it("leaves a pre-existing gate_check_mode='off' row on the new column's default 'disabled'", () => {
49+
const db = applyMigrationsBefore(MIGRATION_FILE);
50+
insertRepositorySettingsRow(db, "acme/legacy-off", "off");
51+
52+
applyMigration(db, MIGRATION_FILE);
53+
54+
expect(readReviewCheckMode(db, "acme/legacy-off")).toBe("disabled");
55+
});
56+
57+
// The backfill's WHERE clause is an exact match on 'enabled' -- any other legacy/unrecognized value (not
58+
// just the documented 'off') must fail closed to the new column's 'disabled' default rather than publish
59+
// the check-run for a repo that never explicitly opted in.
60+
it("leaves a row with an unrecognized legacy gate_check_mode value on the new column's default 'disabled'", () => {
61+
const db = applyMigrationsBefore(MIGRATION_FILE);
62+
insertRepositorySettingsRow(db, "acme/legacy-weird", "weird-legacy-value");
63+
64+
applyMigration(db, MIGRATION_FILE);
65+
66+
expect(readReviewCheckMode(db, "acme/legacy-weird")).toBe("disabled");
67+
});
68+
69+
it("backfills multiple pre-existing rows independently in one migration run", () => {
70+
const db = applyMigrationsBefore(MIGRATION_FILE);
71+
insertRepositorySettingsRow(db, "acme/legacy-enabled-1", "enabled");
72+
insertRepositorySettingsRow(db, "acme/legacy-enabled-2", "enabled");
73+
insertRepositorySettingsRow(db, "acme/legacy-off-1", "off");
74+
75+
applyMigration(db, MIGRATION_FILE);
76+
77+
expect(readReviewCheckMode(db, "acme/legacy-enabled-1")).toBe("required");
78+
expect(readReviewCheckMode(db, "acme/legacy-enabled-2")).toBe("required");
79+
expect(readReviewCheckMode(db, "acme/legacy-off-1")).toBe("disabled");
80+
});
81+
});

0 commit comments

Comments
 (0)