test(daemon): cover the tool-call, account and worktree families in storage - #87
Merged
Merged
Conversation
…torage
storage/mod.rs is the largest remaining gap in the mutation gate: 103
mutants against 17 tests. Three whole families had no coverage at all -
tool calls, accounts and worktrees, including their event logs - so every
mutant in them survived.
Every mutant in those families was hand-applied and the suite confirmed to
fail.
The discipline is the one the file's own header already sets out, and it
matters most here: cargo-mutants replaces an `async fn -> Result<()>` body
with `Ok(())`, and replaces a getter with `Ok(None)` or
`Ok(Some(Default::default()))`. A test that calls the function and asserts
it returned Ok kills none of those. So every write is read back and the
observed value asserted, and every getter is checked against a row it did
not itself fabricate.
Some specifics worth naming:
- complete_tool_call writes output, status and completed_at. Asserting
only the status would let a mutant drop the output binding and pass, so
all three are asserted, plus the identifying fields to catch a mutant
that rewrites the wrong row.
- A failed tool call carries no output but must still be marked complete,
which pins `output` as a nullable binding rather than a skipped one.
- Listing is pinned on both axes: scoped to one session, and ordered
oldest-first with deliberately distinct timestamps so the ordering is
decidable rather than incidental.
- Accounts list by ascending priority, which is what the router relies on
to pick the preferred account. The fixture inserts them out of order so
the ORDER BY is observable.
- set_account_limited is exercised in both directions - set a value and
clear it back to NULL - because a test that only sets it would miss a
mutant that ignores the None case.
- load_worktrees is the startup query and must skip the two terminal
states. The fixture puts one worktree in each of the four statuses, so
the right answer (2) differs from "all four" and from either single
exclusion.
One of these tests was wrong on the first pass, and the gate said so.
`prune_tool_call_events` was covered only at zero days and at a window with
nothing old enough to delete - so both assertions expected 0, and a body
mutated to `Ok(0)` passed, as did inverting the `days == 0` guard. The
replacement backdates two of three events and asserts exactly 2 pruned, a
value that differs from 0, 1 and 3. Both mutants die now. A passing test
proves nothing until the mutant has been applied.
While investigating that I suspected a second bug: prune_tool_call_events
compares an RFC3339 `created_at` against SQLite's `datetime()`, which uses
a space separator rather than 'T', and 'T' sorts above ' '. It turns out
the date prefix is compared first, so the separator only matters within a
single day and `days` is at minimum 1. Tested, not a bug - recorded here
because the reasoning looked convincing and was not.
Still uncovered in this file, and left for follow-up rather than claimed:
push tokens (upsert/list/delete), repo contexts (add/list/remove), and a
handful of session and message accessors. This PR does not pretend
storage/mod.rs is finished.
No production code changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
storage/mod.rsis the largest remaining gap in the mutation gate — 103 mutants against 17 tests. Three whole families had no coverage at all: tool calls, accounts and worktrees, including their event logs. Every mutant in them survived.Every mutant in those families was hand-applied and the suite confirmed to fail: 33 killed, 0 survived.
Why "it returned Ok" proves nothing here
cargo-mutants replaces an
async fn -> Result<()>body withOk(()), and a getter withOk(None)orOk(Some(Default::default())). A test that calls the function and asserts it returnedOkkills none of those. So every write is read back and the observed value asserted, and every getter is checked against a row it did not itself fabricate.Specifics worth naming:
complete_tool_callwrites output, status andcompleted_at. Asserting only the status would let a mutant drop the output binding and pass, so all three are asserted — plus the identifying fields, to catch a mutant that rewrites the wrong row.outputas a nullable binding rather than a skipped one.ORDER BYis observable.set_account_limitedis exercised in both directions — set a value and clear it back to NULL — because a test that only sets it would miss a mutant that ignores theNonecase.load_worktreesis the startup query and must skip the two terminal states. The fixture puts one worktree in each of the four statuses, so the right answer (2) differs from "all four" and from either single exclusion.One of these tests was wrong, and the gate said so
prune_tool_call_eventswas first covered only at zero days and at a window with nothing old enough to delete — so both assertions expected0, and a body mutated toOk(0)passed trivially, as did inverting thedays == 0guard. Two survivors, entirely my fault rather than the code's.The replacement backdates two of three events and asserts exactly 2 pruned — a value distinct from 0, 1 and 3. Both mutants die now. A passing test proves nothing until the mutant has been applied.
A bug I thought I'd found, and hadn't
While investigating, I suspected
prune_tool_call_eventscould never delete anything: it compares an RFC3339created_atagainst SQLite'sdatetime(), which uses a space separator rather thanT, and'T'(0x54) sorts above' '(0x20). The siblingprune_old_sessionscomputes its cutoff in Rust instead, which looked like corroboration.It is not a bug. The date prefix is compared first, so the separator only matters within a single day, and
daysis at minimum 1. The test disproved it. Recorded here because the reasoning was convincing and wrong.Still uncovered
Push tokens (upsert/list/delete), repo contexts (add/list/remove), and a handful of session and message accessors. This PR does not pretend
storage/mod.rsis finished.No production code changes — the diff is one test file.
cargo fmt --check,cargo clippy --all-targets -- -D warningsand the full 893-test lib suite are clean.