test(tasks): cover the agent-task store's claim, sweep and archive logic - #77
Merged
Merged
Conversation
src/tasks/storage.rs (940 lines) had zero tests. Adds 16 covering add_task's defaults, claim_task's exclusivity and re-claim window, update_status's completion-notes guard, interrupt_stale_tasks, archive_done_tasks and prune_activity_log. Time is handled by writing last_heartbeat / completed_at directly with SQL rather than sleeping. A sleep-based test for a 90-second reclaim window would either take 90 seconds or be a lie. The reclaim-window tests straddle the boundary deliberately. The guard is "elapsed > reclaim_window * 2", so with the default 90s window 400s must be rejected and 100s accepted; with an explicit 10s timeout that SAME 100s must now be rejected. That pins the parameter as actually used, killing both "unwrap_or(90) -> a constant" and "* 2 -> * 1". One real gap was found by the spot-check and fixed. prune_activity_log's fixture originally held entries at 3 days and 1 hour with retention=2 days. The "* 86400 -> * 3600" mutant (cutoff 2 HOURS instead of 2 days) deletes exactly the same two rows, so the test passed while killing nothing. Added a 1-day-old entry, the only value that separates the two cutoffs, and asserted the survivors BY ID rather than by count. That mutant now dies. This is why every suite is verified by hand-applying mutants instead of trusting green. Verified 7/7 mutants killed: reclaim window * 2 -> * 1; unwrap_or(90) -> (0); the done-notes guard removed; archive cutoff * 3600 -> * 1; archive's status='done' filter dropped; prune * 86400 -> * 3600; and interrupt_stale_tasks returning an empty vec. Noted, not changed: claim_task's lease branch matches status = 'claimed' and the conflict query matches 'claimed'/'active', but the schema CHECK permits only pending/in_progress/in_cr/in_qa/blocked/interrupted/done/deferred. Those predicates can never match. Left alone - narrowing live SQL is a behaviour change, not test coverage. cargo test --lib tasks::storage::tests passes; clippy --all-targets --all-features -D warnings and cargo fmt --check both exit 0.
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.
src/tasks/storage.rs(940 lines) had zero tests. Adds 16 coveringadd_task's defaults,claim_task's exclusivity and re-claim window,update_status's completion-notes guard,interrupt_stale_tasks,archive_done_tasksandprune_activity_log.Time without sleeping
now_ts()is wall-clock, so the tests writelast_heartbeat/completed_atdirectly with SQL. A sleep-based test for a 90-second reclaim window would either take 90 seconds or be a lie.The reclaim-window tests straddle the boundary deliberately. The guard is
elapsed > reclaim_window * 2:The same 100s flipping verdict with the timeout is what pins the parameter as actually used — killing both
unwrap_or(90) → a constantand* 2 → * 1.A real gap the spot-check caught
prune_activity_log's fixture originally held entries at 3 days and 1 hour withretention=2days. The* 86400 → * 3600mutant moves the cutoff from 2 days to 2 hours — and deletes exactly the same two rows. The test passed while killing nothing.Fixed by adding a 1-day-old entry, the only value that separates the two cutoffs, and asserting the survivors by id rather than by count. That mutant now dies.
This is the whole reason each suite is verified by hand-applying mutants rather than trusting a green run.
Verification — 7/7 killed
* 2→* 1unwrap_or(90)→(0)* 3600→* 1status='done'filter dropped* 86400→* 3600interrupt_stale_tasks→ empty veccargo test --lib tasks::storage::testspasses;clippy --all-targets --all-features -- -D warningsandcargo fmt --checkboth exit 0.Noted, not changed
claim_task's lease branch matchesstatus = 'claimed', and the conflict query matches'claimed'/'active'— but the schema's CHECK constraint permits onlypending/in_progress/in_cr/in_qa/blocked/interrupted/done/deferred. Those predicates can never match. Left alone: narrowing live SQL is a behaviour change, not test coverage. Worth a look separately.