Address review: safer save, stricter load and id parsing
- save() follows symlinks, keeps permissions, uses a unique temp file and cleans it up on failure
- load() rejects entries that aren't {id, text, done}
- add() finds the max id with reduce (no spread limit)
- CLI accepts only plain digits as ids and reports save errors readably
- Ignore *.tmp
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ba17376132
commit
8dc5194438
5 files changed
+95
-13
No files matched your search
+45
-3
@@ -1,8 +1,8 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, writeFileSync, existsSync } from "node:fs";
|
||||
import { mkdtempSync, writeFileSync, readdirSync, symlinkSync, lstatSync, statSync, chmodSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { join, dirname } from "node:path";
|
||||
import { load, save, add, has, complete, remove, format } from "../src/todo.js";
|
||||
|
||||
const tempFile = () => join(mkdtempSync(join(tmpdir(), "todo-")), "todos.json");
|
||||
@@ -42,12 +42,46 @@ test("load returns an empty list for a missing file", () => {
|
||||
assert.deepEqual(load(tempFile()), []);
|
||||
});
|
||||
|
||||
test("add handles a very large list", () => {
|
||||
const todos = Array.from({ length: 200_000 }, (_, i) => ({ id: i + 1, text: "x", done: false }));
|
||||
assert.equal(add(todos, "y").at(-1).id, 200_001);
|
||||
});
|
||||
|
||||
test("save then load round-trips and leaves no temp file", () => {
|
||||
const file = tempFile();
|
||||
const todos = add([], "a");
|
||||
save(file, todos);
|
||||
assert.deepEqual(load(file), todos);
|
||||
assert.equal(existsSync(`${file}.tmp`), false);
|
||||
assert.deepEqual(readdirSync(dirname(file)), ["todos.json"]);
|
||||
});
|
||||
|
||||
test("save writes through a symlink instead of replacing it", () => {
|
||||
const real = tempFile();
|
||||
const link = join(dirname(tempFile()), "link.json");
|
||||
save(real, []);
|
||||
symlinkSync(real, link);
|
||||
save(link, add([], "a"));
|
||||
assert.equal(lstatSync(link).isSymbolicLink(), true);
|
||||
assert.deepEqual(load(real).map((t) => t.text), ["a"]);
|
||||
});
|
||||
|
||||
test("save keeps the file's permissions", () => {
|
||||
const file = tempFile();
|
||||
save(file, []);
|
||||
chmodSync(file, 0o600);
|
||||
save(file, add([], "a"));
|
||||
assert.equal(statSync(file).mode & 0o777, 0o600);
|
||||
});
|
||||
|
||||
test("save reports a readable error and cleans up when it can't write", () => {
|
||||
const file = tempFile();
|
||||
chmodSync(dirname(file), 0o500);
|
||||
try {
|
||||
assert.throws(() => save(file, []), /Could not save .*todos\.json/);
|
||||
assert.deepEqual(readdirSync(dirname(file)), []);
|
||||
} finally {
|
||||
chmodSync(dirname(file), 0o700);
|
||||
}
|
||||
});
|
||||
|
||||
test("load rejects invalid JSON with a readable error", () => {
|
||||
@@ -61,3 +95,11 @@ test("load rejects JSON that is not a list", () => {
|
||||
writeFileSync(file, "{}");
|
||||
assert.throws(() => load(file), /expected a list of todos/);
|
||||
});
|
||||
|
||||
for (const entry of ["null", "1", '{"text":"a","done":false}', '{"id":"1","text":"a","done":false}']) {
|
||||
test(`load rejects an invalid entry: ${entry}`, () => {
|
||||
const file = tempFile();
|
||||
writeFileSync(file, `[${entry}]`);
|
||||
assert.throws(() => load(file), /entry 1 is not a valid todo/);
|
||||
});
|
||||
}
|
||||
Reference in new issue
Block a user