From c7a37f82168169cb47130647b99adb6aa7924c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Mon, 26 Oct 2026 10:54:56 -0300 Subject: [PATCH] Make weak tests fail when their feature breaks A review of the existing tests found eight that would still pass with their feature broken. They now check outcomes after each step, with expected values written out. Only tests changed. - layers.test.ts: the jump test writes its offsets out. The settleAll test checks the layers instead of an id that never exists. The in-place edit test checks after each cycle. The fork and drop-version tests check the saved Markdown after undo, and the fork test also after redo. - lib.rs: the save test reads the file back, and expects the literal "changed_on_disk" that src/lib/files.ts matches. - ai.rs: the request body test compares the whole body, with the model id written out. - claude_code.rs: the arguments test, now builds_the_command_line, compares the whole list, in order. - Each new check failed against a planted bug that the old test missed, then passed with the code restored. - Reviewed with the review-changes skill: a task review and the tests check. Their findings are fixed or left as follow-ups. --- src-tauri/src/ai.rs | 20 ++++++++++++++------ src-tauri/src/claude_code.rs | 31 ++++++++++++++++++++++++------- src-tauri/src/lib.rs | 4 +++- src/editor/layers.test.ts | 28 +++++++++++++++++++--------- 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/src-tauri/src/ai.rs b/src-tauri/src/ai.rs index 0f1dbb0..f798f89 100644 --- a/src-tauri/src/ai.rs +++ b/src-tauri/src/ai.rs @@ -211,12 +211,20 @@ mod tests { #[test] fn builds_a_structured_output_request() { - let body = request_body(&request(), true); - assert_eq!(body["model"], MODEL); - assert_eq!(body["fallbacks"], "default"); - assert_eq!(body["output_config"]["format"]["type"], "json_schema"); - assert_eq!(body["output_config"]["effort"], "low"); - assert!(request_body(&request(), false).get("fallbacks").is_none()); + let without_fallback = json!({ + "model": "claude-opus-5-5", + "max_tokens": 1000, + "system": "sys", + "messages": [{ "role": "user", "content": "hi" }], + "output_config": { + "effort": "low", + "format": { "type": "json_schema", "schema": { "type": "object" } }, + }, + }); + assert_eq!(request_body(&request(), false), without_fallback); + let mut with_fallback = without_fallback; + with_fallback["fallbacks"] = json!("default"); + assert_eq!(request_body(&request(), true), with_fallback); } #[test] diff --git a/src-tauri/src/claude_code.rs b/src-tauri/src/claude_code.rs index 7e8b29d..831aa5b 100644 --- a/src-tauri/src/claude_code.rs +++ b/src-tauri/src/claude_code.rs @@ -277,13 +277,30 @@ mod tests { } #[test] - fn passes_model_effort_and_schema() { - let args = args(&request(), "claude-opus-5-5"); - let after = |flag: &str| &args[args.iter().position(|a| a == flag).unwrap() + 1]; - assert_eq!(after("--model"), "claude-opus-5-5"); - assert_eq!(after("--effort"), "low"); - assert_eq!(after("--json-schema"), r#"{"type":"object"}"#); - assert_eq!(after("--tools"), ""); + fn builds_the_command_line() { + assert_eq!( + args(&request(), "claude-opus-5-5"), + [ + "-p", + "--output-format", + "json", + "--model", + "claude-opus-5-5", + "--effort", + "low", + "--system-prompt", + "sys", + "--json-schema", + r#"{"type":"object"}"#, + "--tools", + "", + "--strict-mcp-config", + "--setting-sources", + "", + "--safe-mode", + "--no-session-persistence", + ] + ); } #[test] diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 2e8c887..99c4423 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -121,11 +121,13 @@ mod tests { // Unchanged since the page read it: the save goes through. assert!(write_text(path.clone(), "edited".into(), Some("mine".into())).is_ok()); + assert_eq!(fs::read_to_string(&file).unwrap(), "edited"); // Another app changed it: refused, and the file keeps their text. + // `src/lib/files.ts` matches this exact error. fs::write(&file, "theirs").unwrap(); assert_eq!( write_text(path.clone(), "again".into(), Some("edited".into())), - Err(CHANGED_ON_DISK.into()) + Err("changed_on_disk".to_string()) ); assert_eq!(fs::read_to_string(&file).unwrap(), "theirs"); // Without `expected` (Keep mine), it overwrites. diff --git a/src/editor/layers.test.ts b/src/editor/layers.test.ts index 57e1a2d..b49cd22 100644 --- a/src/editor/layers.test.ts +++ b/src/editor/layers.test.ts @@ -8,7 +8,6 @@ import { cycleAlt, cycleAtCaret, deleteGhosts, - findAlt, forkVersion, jumpToItem, removeAltVariant, @@ -61,9 +60,13 @@ describe("alternatives", () => { const id = altIdAt(state, "struggle"); const pos = at(state, "struggle") + "struggle".length - 1; state = state.update({ changes: { from: pos, insert: "s" } }).state; + expect(save(state)).toBe("the {{tension|>strugglse}} here\n"); state = apply(state, cycleAlt(state, id, 1)); + expect(state.doc.toString()).toBe("the tension here"); + expect(save(state)).toBe("the {{tension|strugglse}} here\n"); state = apply(state, cycleAlt(state, id, 1)); expect(state.doc.toString()).toBe("the strugglse here"); + expect(save(state)).toBe("the {{tension|>strugglse}} here\n"); }); it("takes letters typed at its end, but not a space or text at its start", () => { @@ -158,8 +161,12 @@ describe("paragraph versions", () => { expect(save(state)).toBe(":::alt\nOriginal paragraph.\n:::or*\nOriginal paragraph. Edited.\n:::\n"); state = apply(state, setVersionActive(state, 0, 0)); expect(state.doc.toString()).toBe("Original paragraph."); + expect(save(state)).toBe(":::alt*\nOriginal paragraph.\n:::or\nOriginal paragraph. Edited.\n:::\n"); state = run(state, undo); expect(state.doc.toString()).toBe("Original paragraph. Edited."); + expect(save(state)).toBe(":::alt\nOriginal paragraph.\n:::or*\nOriginal paragraph. Edited.\n:::\n"); + state = run(state, redo); + expect(save(state)).toBe(":::alt*\nOriginal paragraph.\n:::or\nOriginal paragraph. Edited.\n:::\n"); }); it("brings in the layers of the version it switches to", () => { @@ -174,8 +181,10 @@ describe("paragraph versions", () => { let state = stateFor("Above.\n\n:::alt\nA.\n:::or\nB.\n:::\n"); state = state.update({ changes: { from: 6, to: 8 } }).state; expect(layersOf(state).versions).toEqual([]); + expect(save(state)).toBe("Above.A.\n"); state = run(state, undo); expect(layersOf(state).versions).toHaveLength(1); + expect(save(state)).toBe("Above.\n\n:::alt*\nA.\n:::or\nB.\n:::\n"); }); }); @@ -188,19 +197,20 @@ describe("keyboard navigation", () => { it("jumps forward and back with wrap-around", () => { let state = stateFor(md); - const items = versionedItems(state); + // "one" starts at 6. The last paragraph and its first word, "two", both start at 35. + expect(state.doc.toString()).toBe("Start one here.\n\nPlain paragraph.\n\ntwo and more."); let last: number | null = null; const jump = (dir: 1 | -1) => { const r = jumpToItem(state, dir, last)!; state = apply(state, r.spec); last = r.index; - return state.selection.main.head; + return [state.selection.main.head, r.index]; }; - expect(jump(1)).toBe(items[0].from); - expect(jump(1)).toBe(items[1].from); - expect(jump(1)).toBe(items[2].from); - expect(jump(1)).toBe(items[0].from); - expect(jump(-1)).toBe(items[2].from); + expect(jump(1)).toEqual([6, 0]); + expect(jump(1)).toEqual([35, 1]); + expect(jump(1)).toEqual([35, 2]); + expect(jump(1)).toEqual([6, 0]); + expect(jump(-1)).toEqual([35, 2]); }); it("cycles the word at the caret before its paragraph", () => { @@ -228,6 +238,6 @@ describe("ghosts and counting", () => { let state = stateFor("A {{one|>uno}} word. {-gone-}\n\n:::alt\nFirst.\n:::or*\nSecond.\n:::\n"); state = apply(state, settleAll(state)); expect(save(state)).toBe("A uno word. {-gone-}\n\nSecond.\n"); - expect(findAlt(state, "x")).toBeNull(); + expect(layersOf(state)).toEqual({ alts: [], ghosts: [{ from: 12, to: 16 }], versions: [] }); }); });