diff --git a/.claude/skills/test-standards/SKILL.md b/.claude/skills/test-standards/SKILL.md index 9e583758..c8f1c8f9 100644 --- a/.claude/skills/test-standards/SKILL.md +++ b/.claude/skills/test-standards/SKILL.md @@ -89,10 +89,13 @@ during this review. ## Validation tests -- Tests for parsing/validation pair every malformed-input case with an - explicit control case (comment prefix `Control:`) proving the same parser - accepts valid input; without one, "rejects bad input" is indistinguishable - from "rejects everything". +- A family of related rejection cases (one parser's malformed inputs, one + table's rows, one function's length guards) is one table-driven test whose + rows are the branches. Flag a family spelled out as one test per row, and + flag a control written as a separate test: the table carries at least one + accepting row marked `Control:` beside the rows it controls. A separate + test is warranted only for a branch with its own spec citation or its own + failure mode. - Cover the reject-the-whole-object rule: one bad sibling field must reject the enclosing object, and the test must show neighboring valid fields did not survive into the output. @@ -102,6 +105,14 @@ during this review. - No tests that restate the implementation line by line, duplicate an existing case with cosmetic variation, or exist to inflate a count. Recommending deletion of a weak test is a valid review outcome. +- No assertions on log wording unless the message is the documented + contract. A guard whose only observable is a log line is covered through + the behavior it protects; flag a test that would fail on a reworded + message. +- Assertions target what a caller or peer observes. Flag a test that reads + private state, queue contents, or the identity of the thread that ran a + step when an observable outcome would distinguish the correct path; where + none would, the test states that. - Test names and comments describe the behavior under test, not the defect history ("rejects spectrum config missing n_disp_bins", not "regression test for the config bug"), closely enough that a red CI run identifies the diff --git a/docs/conventions.md b/docs/conventions.md index e8c4ddd4..3e42878e 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -131,9 +131,22 @@ checklists in `.claude/skills/` apply these standards to a diff. clock. - A test defends a specific production line or branch and fails when that line is deleted or its condition inverted. A test that cannot fail that - way is filler and is deleted. Every malformed-input case in a validation - test is paired with a `Control:` case showing the same parser accepts - valid input. + way is filler and is deleted. +- The unit of a test is a behavior, not a branch. A family of related guards + (the rejection cases of one parser, the rows of one admission table, the + length checks of one function) is one table-driven test whose rows are the + branches; a guard gets its own test only when it has its own spec citation + or its own failure mode. Each table carries at least one accepting row as + its control, marked `Control:`, so a rejection is distinguishable from + rejecting everything. Controls live beside what they control, not as + separate tests. +- Tests assert on outcomes, not on log wording. A guard whose only + observable effect is a log line is covered by the test of the behavior it + protects or not at all; a log substring is asserted only where the message + is the documented contract. +- Tests assert on what a caller or peer can observe. Private state, queue + contents, and which thread ran a step are reached only when no observable + outcome distinguishes the correct path, and the test says so. - Elapsed time is never a pass/fail condition. A blocked call is proven by waiting with no timeout, by a value only the correct path can produce, or by a structural failure; hangs are caught by the suite watchdog and the