diff --git a/src/cli.js b/src/cli.js index 90ffe77..85d7f28 100755 --- a/src/cli.js +++ b/src/cli.js @@ -1,28 +1,45 @@ #!/usr/bin/env node -import { load, save, add, complete, remove, format } from "./todo.js"; +import { load, save, add, has, complete, remove, format } from "./todo.js"; const FILE = process.env.TODO_FILE ?? "todos.json"; -const [cmd, ...args] = process.argv.slice(2); -let todos = load(FILE); +const USAGE = "Usage: todo [ls | add | done | rm ]"; +const [cmd = "ls", ...args] = process.argv.slice(2); -switch (cmd) { - case "add": - todos = add(todos, args.join(" ")); - break; - case "done": - todos = complete(todos, Number(args[0])); - break; - case "rm": - todos = remove(todos, Number(args[0])); - break; - case "ls": - case undefined: - console.log(format(todos)); - process.exit(0); - default: - console.error(`Unknown command: ${cmd}\nUsage: todo [ls | add | done | rm ]`); - process.exit(1); +function fail(message) { + console.error(message); + process.exit(1); } -save(FILE, todos); +function parseId(todos, arg) { + const id = Number(arg); + if (arg === undefined || !Number.isInteger(id)) fail(`Invalid id: ${arg ?? "(missing)"}\n${USAGE}`); + if (!has(todos, id)) fail(`No todo with id ${id}`); + return id; +} + +if (!["ls", "add", "done", "rm"].includes(cmd)) fail(`Unknown command: ${cmd}\n${USAGE}`); + +let todos; +try { + todos = load(FILE); +} catch (err) { + fail(err.message); +} + +switch (cmd) { + case "add": { + const text = args.join(" ").trim(); + if (!text) fail(`Missing todo text\n${USAGE}`); + todos = add(todos, text); + break; + } + case "done": + todos = complete(todos, parseId(todos, args[0])); + break; + case "rm": + todos = remove(todos, parseId(todos, args[0])); + break; +} + +if (cmd !== "ls") save(FILE, todos); console.log(format(todos)); diff --git a/src/todo.js b/src/todo.js index efb8343..c9e6e94 100644 --- a/src/todo.js +++ b/src/todo.js @@ -1,19 +1,33 @@ -import { readFileSync, writeFileSync, existsSync } from "node:fs"; +import { readFileSync, writeFileSync, renameSync, existsSync } from "node:fs"; export function load(path) { if (!existsSync(path)) return []; - return JSON.parse(readFileSync(path, "utf8")); + let todos; + try { + todos = JSON.parse(readFileSync(path, "utf8")); + } catch (err) { + throw new Error(`Could not read ${path}: ${err.message}`); + } + if (!Array.isArray(todos)) throw new Error(`Could not read ${path}: expected a list of todos`); + return todos; } +// Write to a temp file and rename, so an interrupted write never leaves `path` truncated. export function save(path, todos) { - writeFileSync(path, JSON.stringify(todos, null, 2) + "\n"); + const tmp = `${path}.tmp`; + writeFileSync(tmp, JSON.stringify(todos, null, 2) + "\n"); + renameSync(tmp, path); } export function add(todos, text) { - const id = todos.length + 1; + const id = Math.max(0, ...todos.map((t) => t.id)) + 1; return [...todos, { id, text, done: false }]; } +export function has(todos, id) { + return todos.some((t) => t.id === id); +} + export function complete(todos, id) { return todos.map((t) => (t.id === id ? { ...t, done: true } : t)); } diff --git a/test/cli.test.js b/test/cli.test.js new file mode 100644 index 0000000..4e73029 --- /dev/null +++ b/test/cli.test.js @@ -0,0 +1,69 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, writeFileSync, readFileSync, existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const CLI = fileURLToPath(new URL("../src/cli.js", import.meta.url)); + +function run(file, ...args) { + return spawnSync(process.execPath, [CLI, ...args], { + env: { ...process.env, TODO_FILE: file }, + encoding: "utf8", + }); +} + +const tempFile = () => join(mkdtempSync(join(tmpdir(), "todo-")), "todos.json"); + +test("ls with no file prints the empty message and writes nothing", () => { + const file = tempFile(); + const res = run(file, "ls"); + assert.equal(res.status, 0); + assert.equal(res.stdout.trim(), "Nothing to do."); + assert.equal(existsSync(file), false); +}); + +test("add with no text fails without saving", () => { + const file = tempFile(); + const res = run(file, "add", " "); + assert.equal(res.status, 1); + assert.match(res.stderr, /Missing todo text/); + assert.equal(existsSync(file), false); +}); + +for (const [label, args, message] of [ + ["missing id", [], /Invalid id: \(missing\)/], + ["non-numeric id", ["abc"], /Invalid id: abc/], + ["unknown id", ["99"], /No todo with id 99/], +]) { + for (const cmd of ["done", "rm"]) { + test(`${cmd} with ${label} fails and leaves the file unchanged`, () => { + const file = tempFile(); + run(file, "add", "a"); + const before = readFileSync(file, "utf8"); + const res = run(file, cmd, ...args); + assert.equal(res.status, 1); + assert.match(res.stderr, message); + assert.equal(readFileSync(file, "utf8"), before); + }); + } +} + +test("unknown command prints usage even when the file is corrupt", () => { + const file = tempFile(); + writeFileSync(file, "{ not json"); + const res = run(file, "--help"); + assert.equal(res.status, 1); + assert.match(res.stderr, /Unknown command: --help\nUsage:/); +}); + +test("a corrupt file gives a readable error instead of a stack trace", () => { + const file = tempFile(); + writeFileSync(file, "{ not json"); + const res = run(file, "ls"); + assert.equal(res.status, 1); + assert.match(res.stderr, /^Could not read /); + assert.doesNotMatch(res.stderr, /at .*\.js:\d+/); +}); diff --git a/test/todo.test.js b/test/todo.test.js index 01687e7..04beccd 100644 --- a/test/todo.test.js +++ b/test/todo.test.js @@ -1,12 +1,28 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { add, complete, remove, format } from "../src/todo.js"; +import { mkdtempSync, writeFileSync, existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { load, save, add, has, complete, remove, format } from "../src/todo.js"; + +const tempFile = () => join(mkdtempSync(join(tmpdir(), "todo-")), "todos.json"); test("add appends an open todo", () => { const todos = add([], "buy milk"); assert.deepEqual(todos, [{ id: 1, text: "buy milk", done: false }]); }); +test("add after a remove does not reuse an existing id", () => { + const todos = add(remove(add(add([], "a"), "b"), 1), "c"); + assert.deepEqual(todos.map((t) => t.id), [2, 3]); +}); + +test("has reports whether an id exists", () => { + const todos = add([], "a"); + assert.equal(has(todos, 1), true); + assert.equal(has(todos, 2), false); +}); + test("complete marks only the matching todo", () => { const todos = complete(add(add([], "a"), "b"), 2); assert.equal(todos[0].done, false); @@ -21,3 +37,27 @@ test("remove drops the matching todo", () => { test("format renders an empty list", () => { assert.equal(format([]), "Nothing to do."); }); + +test("load returns an empty list for a missing file", () => { + assert.deepEqual(load(tempFile()), []); +}); + +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); +}); + +test("load rejects invalid JSON with a readable error", () => { + const file = tempFile(); + writeFileSync(file, "{ not json"); + assert.throws(() => load(file), /Could not read .*todos\.json/); +}); + +test("load rejects JSON that is not a list", () => { + const file = tempFile(); + writeFileSync(file, "{}"); + assert.throws(() => load(file), /expected a list of todos/); +});