Skip to content

Commit 9617a9b

Browse files
committed
test: improve patch coverage for readonly db handling
Add fix command tests: - Schema check failure with no permission issues sets exitCode=1 - Dry-run with schema failure sets exitCode=1 - Schema check suppresses error when permission issues explain failure - Schema repair success path (missing columns → repaired) Add telemetry test: - Readonly warning fallback when auto-repair fails (mock chmodSync to simulate unowned files)
1 parent c245be5 commit 9617a9b

File tree

2 files changed

+104
-0
lines changed

2 files changed

+104
-0
lines changed

test/commands/cli/fix.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,4 +299,73 @@ describe("sentry cli fix", () => {
299299
expect(stdout).toContain("repaired successfully");
300300
expect(exitCode).toBe(0);
301301
});
302+
303+
test("repairs missing columns and reports success", async () => {
304+
// Create database with pre-migration tables then repair (non-dry-run)
305+
// This exercises the schema repair success path
306+
const db = new Database(join(getTestDir(), "cli.db"));
307+
createPreMigrationDatabase(db);
308+
db.close();
309+
310+
const { stdout, exitCode } = await runFix(false);
311+
312+
expect(stdout).toContain("schema issue(s)");
313+
expect(stdout).toContain("Missing column");
314+
expect(stdout).toContain("Repairing schema");
315+
expect(stdout).toContain("Added column");
316+
expect(stdout).toContain("repaired successfully");
317+
expect(exitCode).toBe(0);
318+
});
319+
320+
test("sets exitCode=1 when schema check throws with no permission issues", async () => {
321+
// Create a DB file that cannot be opened by getRawDatabase.
322+
// Write garbage so SQLite cannot parse it — getRawDatabase will throw.
323+
const dbPath = join(getTestDir(), "cli.db");
324+
closeDatabase();
325+
await Bun.write(dbPath, "not a sqlite database");
326+
chmodSync(dbPath, 0o600);
327+
chmodSync(getTestDir(), 0o700);
328+
329+
const { stdout, stderr, exitCode } = await runFix(false);
330+
331+
// No permission issues found, so schema failure should be reported
332+
expect(stderr).toContain("Could not open database to check schema");
333+
expect(stderr).toContain("Try deleting the database");
334+
expect(exitCode).toBe(1);
335+
// Should NOT say "No issues found"
336+
expect(stdout).not.toContain("No issues found");
337+
});
338+
339+
test("dry-run sets exitCode=1 when schema check throws", async () => {
340+
// Same corrupt DB scenario, but in dry-run mode
341+
const dbPath = join(getTestDir(), "cli.db");
342+
closeDatabase();
343+
await Bun.write(dbPath, "not a sqlite database");
344+
chmodSync(dbPath, 0o600);
345+
chmodSync(getTestDir(), 0o700);
346+
347+
const { stdout, stderr, exitCode } = await runFix(true);
348+
349+
expect(stderr).toContain("Could not open database to check schema");
350+
expect(exitCode).toBe(1);
351+
// Should NOT suggest running fix (no fixable issues found)
352+
expect(stdout).not.toContain("Run 'sentry cli fix' to apply fixes");
353+
});
354+
355+
test("schema check failure with permission issues does not print schema error", async () => {
356+
// When permissions are broken AND schema can't be opened, the schema error
357+
// is suppressed because permission issues are the likely root cause.
358+
getDatabase();
359+
const dbPath = join(getTestDir(), "cli.db");
360+
361+
// Make DB readonly — will cause permission issue AND potentially schema failure
362+
chmodSync(dbPath, 0o444);
363+
364+
// The schema catch block should suppress the error message when perm.found > 0
365+
const { stdout, stderr } = await runFix(true);
366+
367+
expect(stdout).toContain("permission issue(s)");
368+
// Should NOT print "Could not open database" since permission issues explain it
369+
expect(stderr).not.toContain("Could not open database");
370+
});
302371
});

test/lib/telemetry.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,5 +869,40 @@ describe("createTracedDatabase", () => {
869869

870870
db.close();
871871
});
872+
873+
test("shows readonly warning when auto-repair fails", () => {
874+
// Mock chmodSync to always throw, simulating a file owned by another user.
875+
// This makes tryRepairReadonly fail and fall through to warnReadonlyDatabaseOnce.
876+
const { mock: mockFn } = require("bun:test");
877+
mockFn.module("node:fs", () => {
878+
const realFs = require("node:fs");
879+
return {
880+
...realFs,
881+
chmodSync: () => {
882+
throw Object.assign(new Error("EPERM: operation not permitted"), {
883+
code: "EPERM",
884+
});
885+
},
886+
};
887+
});
888+
889+
const db = new Database(dbPath);
890+
const tracedDb = createTracedDatabase(db);
891+
892+
const stderrSpy = spyOn(process.stderr, "write");
893+
894+
tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(2, "Bob");
895+
896+
const output = stderrSpy.mock.calls.map((c) => String(c[0])).join("");
897+
// When repair fails, the warning message should appear instead
898+
expect(output).toContain("read-only");
899+
expect(output).toContain("sentry cli fix");
900+
901+
stderrSpy.mockRestore();
902+
db.close();
903+
904+
// Restore mock
905+
mockFn.module("node:fs", () => require("node:fs"));
906+
});
872907
});
873908
});

0 commit comments

Comments
 (0)