Skip to content

Make weak tests fail when their feature breaks - #8

Merged
gnapse merged 1 commit into
mainfrom
tests/tighten-weak-tests
Oct 26, 2026
Merged

gnapse merged 1 commit into
mainfrom
tests/tighten-weak-tests

Conversation

@gnapse

@gnapse gnapse commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

A review of the existing tests found eight that would still pass with their feature broken. This PR makes each one fail when its feature breaks. Only tests changed. No real bug turned up.

What changed

  • src/editor/layers.test.ts
    • The jump test writes its offsets (6, 35) out and checks the page text they refer to. Before, it took them from versionedItems, which jumpToItem itself calls.
    • The settleAll test checks the layers. Before, it checked findAlt(state, "x"), an id that never exists.
    • The in-place edit test checks the text and saved Markdown after each cycle, not only at the end.
    • The fork and drop-version tests check the saved Markdown after undo. The fork test also checks it after redo.
  • src-tauri/src/lib.rs: the save test reads the file back. It expects the literal "changed_on_disk" that src/lib/files.ts matches, not the code's own constant.
  • src-tauri/src/ai.rs: the request body test compares the whole body: model, system, prompt, max_tokens, effort, and schema.
  • src-tauri/src/claude_code.rs: the arguments test, renamed to builds_the_command_line, compares the whole list.

Evidence

Each bug was planted in the code. The old test (from main) and the new one ran against it, then the code was restored.

# Planted bug Old test New test
1 versionedItems puts alternatives at their end (from: a.to) passes fails: [9, 0], expected [6, 0]
2 settleAll leaves one-variant alternatives passes fails: layersOf still has alternatives
3 cycleAlt does nothing ((alt.active + n) % n) passes fails: the strugglse here, expected the tension here
4a undo resets a version's active to 0 passes fails: saved Markdown stars the wrong version
4b undo keeps only the shown version passes fails: saved Above.\n\nA.\n
4c redo doesn't restore the layers passes fails at the redo check (layers.test.ts:169)
5 write_text returns Ok without writing when the file is unchanged passes fails: file holds "mine", expected "edited"
6 CHANGED_ON_DISK renamed to "changed-on-disk" passes fails: Err("changed-on-disk")
7a–c max_tokens fixed at 4096 / prompt sent as system / schema dropped passes fails: the body differs in that field
8a–c --system-prompt / --safe-mode / --strict-mcp-config removed passes fails: the list lacks that flag

Two fresh reviewers ran with the review-changes skill: a task review and the .agents/checks/tests.md check. Both passed the change, and between them they reproduced rows 1–6, 7b, and 8b. pnpm verify passes.

Choices to look at

  • ai.rs writes the model id claude-opus-5-5 out. Changing MODEL now means changing this test too. That is on purpose: the API relies on the value.
  • claude_code.rs checks the exact flag list, in order. It catches a flag that was added (say, one that turns tools back on), not only one that was removed. The cost: reordering the flags breaks the test.

Follow-ups (not in this PR)

  • No test undoes and redoes forkVersion, settleAll, removeAltVariant, or deleteGhosts, though CLAUDE.md asks for one for each command that changes layers. A reviewer planted addToHistory.of(false) in forkVersion and in settleAll, and all tests still passed.
  • No test cycles backward (cycleAlt or cycleAtCaret with -1), which ⌥K uses. A cycleAlt that always steps forward passes every test. A backward test needs three or more variants.

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.
@gnapse
gnapse force-pushed the tests/tighten-weak-tests branch from 640275a to c7a37f8 Compare October 26, 2026 14:40
@gnapse
gnapse merged commit c7a37f8 into main Oct 26, 2026
1 check passed
@gnapse
gnapse deleted the tests/tighten-weak-tests branch October 26, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant