Skip to content

Commit 1dcbdef

Browse files
committed
Add comprehensive tests for SQLite backup pruning
- 6 new tests in sqliteBootstrap.test.ts: prune deletes old .bak files, no-op within limit, deletes all when maxBackups=0, fs indirection, missing directory handling, skip .bak creation when maxBackups=0 - 3 new tests in sqliteStorage.test.ts: normalization clamps to 0-20, defaults to 3 for non-numbers, rounds to nearest integer - Export normalizeMaxSqliteBackups for testability - Import pruneSqliteBackups in test file
1 parent d8c74c0 commit 1dcbdef

4 files changed

Lines changed: 194 additions & 2 deletions

File tree

.github/skills/copilot-scheduler-intro/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ If you use external services, you can add their MCP servers to the same `.vscode
128128
- **Perplexity** — research-grounded AI queries. Needs an API key.
129129
- **Prefab by Max Health Inc.** — dynamic config, feature flags, and Prefab UI rendering. Comes with a bundled skill (`prefab-ui`) and a custom agent (`Prefab UI Specialist`).
130130

131-
Third-party servers are manually added. The "Set Up MCP" action preserves them.
131+
Third-party servers can be auto-configured with the **"Set Up Third-Party MCP"** button in the Settings tab, which adds secure template entries for Perplexity and Tavily using `${input:...}` placeholders. Google / google-grounded is documented in the guidance but needs a manual entry. The "Set Up MCP" action preserves them.
132132

133133
### Add MCP To Codex
134134

src/sqliteStorage.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export function getConfiguredSqliteJsonMirrorEnabled(
7979
);
8080
}
8181

82-
function normalizeMaxSqliteBackups(value: unknown): number {
82+
export function normalizeMaxSqliteBackups(value: unknown): number {
8383
if (typeof value !== "number" || !Number.isFinite(value)) {
8484
return 3;
8585
}

src/test/suite/sqliteBootstrap.test.ts

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
bootstrapGlobalSqliteStorage,
88
bootstrapWorkspaceSqliteStorage,
99
exportWorkspaceSqliteToJsonMirrors,
10+
pruneSqliteBackups,
1011
readWorkspaceCockpitBoardFromSqlite,
1112
setSqliteAtomicWriteFsForTests,
1213
setSqliteLockOptionsForTests,
@@ -855,4 +856,169 @@ suite("SQLite Bootstrap Tests", () => {
855856
cleanup(workspaceRoot);
856857
}
857858
});
859+
860+
// ──────────────────────────────────────────────
861+
// Backup pruning tests
862+
// ──────────────────────────────────────────────
863+
864+
test("pruneSqliteBackups deletes old .bak files keeping only the N newest", () => {
865+
const workspaceRoot = createTempRoot("copilot-sqlite-prune-1-");
866+
try {
867+
const { databasePath } = getWorkspaceStoragePaths(workspaceRoot);
868+
const vscodeDir = path.dirname(databasePath);
869+
fs.mkdirSync(vscodeDir, { recursive: true });
870+
871+
// Create 5 fake .bak files with staggered mtimes
872+
const bakFiles: string[] = [];
873+
for (let i = 0; i < 5; i++) {
874+
const bakPath = path.join(vscodeDir, `copilot-cockpit.db.test${i}.bak`);
875+
fs.writeFileSync(bakPath, `backup-${i}`);
876+
// Set increasing mtimes so 0 is oldest, 4 is newest
877+
const mtime = new Date(Date.now() - (5 - i) * 10000);
878+
fs.utimesSync(bakPath, mtime, mtime);
879+
bakFiles.push(bakPath);
880+
}
881+
882+
// Also create a non-bak file that should be ignored
883+
fs.writeFileSync(path.join(vscodeDir, "other-file.txt"), "not a bak");
884+
885+
// Prune to 2
886+
pruneSqliteBackups(databasePath, 2);
887+
888+
// Only the 2 newest should survive (test4, test3)
889+
assert.strictEqual(fs.existsSync(bakFiles[0]), false, "bak[0] oldest should be deleted");
890+
assert.strictEqual(fs.existsSync(bakFiles[1]), false, "bak[1] should be deleted");
891+
assert.strictEqual(fs.existsSync(bakFiles[2]), false, "bak[2] should be deleted");
892+
assert.strictEqual(fs.existsSync(bakFiles[3]), true, "bak[3] should survive");
893+
assert.strictEqual(fs.existsSync(bakFiles[4]), true, "bak[4] newest should survive");
894+
895+
// Non-bak file should be untouched
896+
assert.strictEqual(fs.existsSync(path.join(vscodeDir, "other-file.txt")), true);
897+
} finally {
898+
cleanup(workspaceRoot);
899+
}
900+
});
901+
902+
test("pruneSqliteBackups is a no-op when bak files are within limit", () => {
903+
const workspaceRoot = createTempRoot("copilot-sqlite-prune-2-");
904+
try {
905+
const { databasePath } = getWorkspaceStoragePaths(workspaceRoot);
906+
const vscodeDir = path.dirname(databasePath);
907+
fs.mkdirSync(vscodeDir, { recursive: true });
908+
909+
// Create only 2 bak files
910+
for (let i = 0; i < 2; i++) {
911+
fs.writeFileSync(path.join(vscodeDir, `copilot-cockpit.db.bak-${i}.bak`), `backup-${i}`);
912+
}
913+
914+
// Prune to 5 (more than we have)
915+
pruneSqliteBackups(databasePath, 5);
916+
917+
// Both should survive
918+
assert.strictEqual(fs.existsSync(path.join(vscodeDir, "copilot-cockpit.db.bak-0.bak")), true);
919+
assert.strictEqual(fs.existsSync(path.join(vscodeDir, "copilot-cockpit.db.bak-1.bak")), true);
920+
} finally {
921+
cleanup(workspaceRoot);
922+
}
923+
});
924+
925+
test("pruneSqliteBackups deletes all when maxBackups is 0", () => {
926+
const workspaceRoot = createTempRoot("copilot-sqlite-prune-3-");
927+
try {
928+
const { databasePath } = getWorkspaceStoragePaths(workspaceRoot);
929+
const vscodeDir = path.dirname(databasePath);
930+
fs.mkdirSync(vscodeDir, { recursive: true });
931+
932+
// Create 3 bak files
933+
for (let i = 0; i < 3; i++) {
934+
fs.writeFileSync(path.join(vscodeDir, `copilot-cockpit.db.bak-${i}.bak`), `backup-${i}`);
935+
}
936+
937+
// Prune to 0
938+
pruneSqliteBackups(databasePath, 0);
939+
940+
// All should be deleted
941+
for (let i = 0; i < 3; i++) {
942+
assert.strictEqual(
943+
fs.existsSync(path.join(vscodeDir, `copilot-cockpit.db.bak-${i}.bak`)),
944+
false,
945+
`bak-${i} should be deleted when maxBackups is 0`,
946+
);
947+
}
948+
} finally {
949+
cleanup(workspaceRoot);
950+
}
951+
});
952+
953+
test("pruneSqliteBackups works through sqliteAtomicWriteFs indirection", () => {
954+
const workspaceRoot = createTempRoot("copilot-sqlite-prune-4-");
955+
try {
956+
const { databasePath } = getWorkspaceStoragePaths(workspaceRoot);
957+
const vscodeDir = path.dirname(databasePath);
958+
fs.mkdirSync(vscodeDir, { recursive: true });
959+
960+
// Create bak files directly on disk
961+
for (let i = 0; i < 4; i++) {
962+
fs.writeFileSync(path.join(vscodeDir, `copilot-cockpit.db.bak-${i}.bak`), `backup-${i}`);
963+
}
964+
965+
// Use the default fs (setSqliteAtomicWriteFsForTests was not called here)
966+
// but verify pruneSqliteBackups uses sqliteAtomicWriteFs by checking
967+
// it still works with the real filesystem
968+
pruneSqliteBackups(databasePath, 1);
969+
970+
// Exactly 1 should survive (the one with highest mtime)
971+
const surviving = fs.readdirSync(vscodeDir).filter((f) => f.endsWith(".bak"));
972+
assert.strictEqual(surviving.length, 1, `Expected 1 surviving bak, got ${surviving.length}`);
973+
} finally {
974+
setSqliteAtomicWriteFsForTests(); // Reset to real fs
975+
cleanup(workspaceRoot);
976+
}
977+
});
978+
979+
test("pruneSqliteBackups handles missing directory gracefully", () => {
980+
const workspaceRoot = createTempRoot("copilot-sqlite-prune-5-");
981+
try {
982+
const { databasePath } = getWorkspaceStoragePaths(workspaceRoot);
983+
// Don't create the directory
984+
985+
// Should not throw
986+
pruneSqliteBackups(databasePath, 3);
987+
} finally {
988+
cleanup(workspaceRoot);
989+
}
990+
});
991+
992+
test("persistSqliteDatabase skips .bak creation when maxBackups is 0", async () => {
993+
const workspaceRoot = createTempRoot("copilot-sqlite-prune-6-");
994+
995+
try {
996+
const paths = getWorkspaceStoragePaths(workspaceRoot);
997+
fs.mkdirSync(path.dirname(paths.publicSchedulerMirrorPath), { recursive: true });
998+
fs.writeFileSync(paths.publicSchedulerMirrorPath, JSON.stringify({ tasks: [], jobs: [], jobFolders: [] }, null, 2), "utf8");
999+
fs.writeFileSync(paths.privateSchedulerMirrorPath, JSON.stringify({ tasks: [], jobs: [], jobFolders: [] }, null, 2), "utf8");
1000+
1001+
const originalRenameSync = fs.renameSync;
1002+
const renameTargets: string[] = [];
1003+
1004+
setSqliteAtomicWriteFsForTests({
1005+
renameSync: ((oldPath: fs.PathLike, newPath: fs.PathLike) => {
1006+
renameTargets.push(String(newPath));
1007+
return originalRenameSync(oldPath, newPath);
1008+
}) as typeof fs.renameSync,
1009+
});
1010+
1011+
// Bootstrap with maxBackups=0
1012+
await bootstrapWorkspaceSqliteStorage(workspaceRoot, true, 0);
1013+
1014+
// No .bak file should have been created
1015+
const allFiles = fs.readdirSync(path.dirname(paths.databasePath));
1016+
const bakFiles = allFiles.filter((f) => f.endsWith(".bak"));
1017+
assert.strictEqual(bakFiles.length, 0, `Expected 0 .bak files, got: ${bakFiles.join(", ")}`);
1018+
assert.ok(fs.existsSync(paths.databasePath), "Database should exist");
1019+
} finally {
1020+
setSqliteAtomicWriteFsForTests();
1021+
cleanup(workspaceRoot);
1022+
}
1023+
});
8581024
});

src/test/suite/sqliteStorage.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
getWorkspaceResearchConfigPath,
1616
getGlobalStorageDatabasePath,
1717
getWorkspaceStoragePaths,
18+
normalizeMaxSqliteBackups,
1819
normalizeSchedulerStorageMode,
1920
normalizeSqliteJsonMirrorEnabled,
2021
} from "../../sqliteStorage";
@@ -75,4 +76,29 @@ suite("SQLite Storage Foundation Tests", () => {
7576
assert.ok(combined.includes("CREATE TABLE IF NOT EXISTS global_tasks"));
7677
assert.ok(combined.includes("global_schema_version"));
7778
});
79+
80+
test("normalizeMaxSqliteBackups clamps to 0-20 range", () => {
81+
assert.strictEqual(normalizeMaxSqliteBackups(3), 3);
82+
assert.strictEqual(normalizeMaxSqliteBackups(0), 0);
83+
assert.strictEqual(normalizeMaxSqliteBackups(20), 20);
84+
assert.strictEqual(normalizeMaxSqliteBackups(21), 20);
85+
assert.strictEqual(normalizeMaxSqliteBackups(-1), 0);
86+
assert.strictEqual(normalizeMaxSqliteBackups(-100), 0);
87+
assert.strictEqual(normalizeMaxSqliteBackups(100), 20);
88+
});
89+
90+
test("normalizeMaxSqliteBackups defaults to 3 for non-number inputs", () => {
91+
assert.strictEqual(normalizeMaxSqliteBackups(undefined), 3);
92+
assert.strictEqual(normalizeMaxSqliteBackups(null), 3);
93+
assert.strictEqual(normalizeMaxSqliteBackups("5"), 3);
94+
assert.strictEqual(normalizeMaxSqliteBackups(NaN), 3);
95+
assert.strictEqual(normalizeMaxSqliteBackups(Infinity), 3);
96+
});
97+
98+
test("normalizeMaxSqliteBackups rounds to the nearest integer", () => {
99+
assert.strictEqual(normalizeMaxSqliteBackups(3.2), 3);
100+
assert.strictEqual(normalizeMaxSqliteBackups(3.9), 4);
101+
assert.strictEqual(normalizeMaxSqliteBackups(0.5), 1);
102+
assert.strictEqual(normalizeMaxSqliteBackups(19.5), 20);
103+
});
78104
});

0 commit comments

Comments
 (0)