From 94d1ce5a5556d0fd1d66d938df0e45e27205f28c Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 26 May 2026 12:45:47 -0700 Subject: [PATCH 1/6] agents|feat: Add test-structure-audit-checklist partial Add a new partial mirroring comment-audit-checklist's shape. The checklist encodes the pre-completion audit for spec files: scan adjacent runs of three or more nearly-identical tests, and refactor when the variation is buried behind shared setup. The partial has no consumer yet; the test-structure section in testing-conventions/SKILL.md is added in a follow-up commit that includes it. --- .../_partials/test-structure-audit-checklist.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 packages/agents/content/_partials/test-structure-audit-checklist.md diff --git a/packages/agents/content/_partials/test-structure-audit-checklist.md b/packages/agents/content/_partials/test-structure-audit-checklist.md new file mode 100644 index 00000000..d5ccce28 --- /dev/null +++ b/packages/agents/content/_partials/test-structure-audit-checklist.md @@ -0,0 +1,11 @@ +Before saving a spec file, scan adjacent tests for shared-setup runs. + +**The diagnostic.** For each run of three or more nearly-identical tests, ask: How many tokens does a reader have to diff to find what's actually different between them? If the answer is more than a handful, the signal is buried. Refactor. + +- **Parameterize** with `it.each` when the variation is small and the test body is structurally identical. +- **Extract a named helper** when the variation is bigger, when shared setup should disappear into a helper signature, or when assertion shapes differ per case. +- **Check the assertions, too.** When the rule is a count or predicate, specific-fixture-label assertions should not repeat per row. Assert specific data flow *once* in a focused test; assert structurally for the rest. + +Do not collapse distinct intents. The smell is shared *setup with one variable*, not shared *shape with different intents*. Three tests that read as genuinely distinct behavioral claims stay as three `it` blocks even if their bodies superficially resemble each other. + +Three or more `it` blocks with identical setup and a single varying input is the most common failure mode in interactive sessions. That material belongs behind a helper or a parameterized table, not in N copies of the same shape. From 8cd55474cbd989e799e16ecbfde5f383e2036dff Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 26 May 2026 12:45:58 -0700 Subject: [PATCH 2/6] agents|feat: Add test-structure section to testing-conventions skill Add three rules, a diagnostic, and a "when N copies are right" carve-out covering signal-to-noise as the bar for parameterization and helpers. The section includes the audit-checklist partial at the end and cross-links to comment-discipline.md as the structurally related signal-buried-in-noise sibling. Specifies that test bodies should be mostly variation and assertion (not setup), that it.each fits when the variation is small and the body is structurally identical (helpers fit otherwise), and that assertions test the rule (counts, predicates) rather than enumerating specific fixture labels per row. The carve-out preserves cases where tests share shape with distinct behavioral intents. --- .../skills/testing-conventions/SKILL.md | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/agents/content/skills/testing-conventions/SKILL.md b/packages/agents/content/skills/testing-conventions/SKILL.md index abf69da7..a8ecbc44 100644 --- a/packages/agents/content/skills/testing-conventions/SKILL.md +++ b/packages/agents/content/skills/testing-conventions/SKILL.md @@ -67,6 +67,28 @@ Comment discipline applies to test files the same as to source. The full rule se Everything else is over-commenting. Test names already communicate intent; assertions communicate the check. +## Test structure + +Tests should make their variation easy to see. When N adjacent tests differ only in one input but share a wall of identical setup, the reader has to diff three or four nearly-identical render calls to find what's actually being tested. The fix is not "delete things"; it's "factor the shared part out so the variation reads as variation." + +This is the same signal-buried-in-noise failure mode that [`comment-discipline.md`](../_data/comment-discipline.md) addresses on the comment side. Different mechanism, same principle. + +### Rules + +1. **Test bodies should be mostly variation and assertion, not setup.** Any prop, fixture, or boilerplate identical across N adjacent tests is noise. Factor it into a default-bearing helper, or collapse the tests into a single parameterized test where the variation reads as a table. +2. **Use `it.each` when N adjacent tests differ only in a small set of inputs and the body is structurally identical.** Use a named helper when the variation is bigger, when shared setup should disappear into a helper signature, or when assertion shapes differ per case. +3. **Test the rule, not the data.** Prefer counts, predicates, and structural assertions (`expect(getVisibleChipCount()).toBe(N)`, `expect(queryPillElement()).toBeInTheDocument()`) over enumerating specific fixture labels in every row. Assert specific data flow _once_, in a focused test, not per row of a table. + +### Diagnostic + +Before writing a third test in the same `describe` block, scan the previous two: How many tokens does a reader have to diff to find what's actually different between them? If the answer is more than a handful, the signal is buried. Parameterize the trio or extract a helper before continuing. + +### When N copies are right + +The smell is shared _setup with one variable_, not shared _shape with different intents_. Three tests that read as genuinely distinct behavioral claims should stay as three `it` blocks even if their bodies superficially resemble each other. Do not collapse distinct intents into a table. + + + ## Additional patterns ### Omit "should" from test names From 16e4b268381b46d930ddea2d35db2a0ebdc84c70 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 26 May 2026 12:46:08 -0700 Subject: [PATCH 3/6] agents|refactor: Point code-patterns at testing-conventions for test structure Replace the one-line it.each bullet under code-patterns' Test structure subsection with a pointer to testing-conventions/SKILL.md, the new canonical source for parameterization, helpers, and signal-to-noise rules. Keeps the describe-as-function reference and __tests__/ sibling-placement bullets, which cover different ground. --- packages/agents/content/skills/code-patterns/SKILL.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/agents/content/skills/code-patterns/SKILL.md b/packages/agents/content/skills/code-patterns/SKILL.md index 7294cee4..3151a8be 100644 --- a/packages/agents/content/skills/code-patterns/SKILL.md +++ b/packages/agents/content/skills/code-patterns/SKILL.md @@ -60,7 +60,8 @@ Follow the naming rules in [naming-conventions.md](../_data/naming-conventions.m ### Test structure -- **Use parameterized tests** with `it.each([...])` to avoid verbose, repetitive test cases +Test-structure rules (parameterization, helpers, signal-to-noise) live in [`testing-conventions/SKILL.md`](../testing-conventions/SKILL.md). + - **Use function/class reference as describe argument** - `describe(myFunction, ...)` instead of `describe('myFunction', ...)` - **Place tests in `__tests__` directory** as sibling to the file being tested From ea13b9f9ecc0a9f5904640b565d0d54e6e2866e1 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 26 May 2026 12:46:16 -0700 Subject: [PATCH 4/6] agents|feat: Extend reviewer to flag test-structure violations Add a Test-structure violations bullet group under Scope in code-simplification-reviewer, after the existing comment-discipline group. The reviewer now flags adjacent tests with near-identical setup, specific-fixture-label assertions repeated per row when the rule is structural, and helpers whose parameters the test bodies still re-specify. Adds severity guidance: a handful of redundant adjacent-test runs is low; pervasive shared-setup duplication across a spec file is medium. The rule source itself is not duplicated here; the reviewer points to testing-conventions/SKILL.md. --- .../content/subagents/code-simplification-reviewer.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/agents/content/subagents/code-simplification-reviewer.md b/packages/agents/content/subagents/code-simplification-reviewer.md index 2491b745..6cbff4dc 100644 --- a/packages/agents/content/subagents/code-simplification-reviewer.md +++ b/packages/agents/content/subagents/code-simplification-reviewer.md @@ -90,6 +90,10 @@ Focus exclusively on simplification opportunities in changed code: - Domain leaks in shared/common code - Inline "what" comments that describe what the code does instead of explaining why a non-obvious decision was made - `eslint-disable` rationales that explain the surrounding decision rather than the specific suppression +- Test-structure violations. See `{platform_home_dir}/skills/testing-conventions/SKILL.md` for the full rule set. Common patterns to flag: + - Adjacent tests with near-identical setup where only one input varies (parameterize with `it.each` or extract a helper) + - Specific-fixture-label assertions repeated per row when the rule is a count or predicate + - Helpers whose parameters the test bodies still re-specify (the abstraction failed to absorb the duplication) - Logic that can be consolidated without sacrificing clarity ### Simplification principles @@ -125,8 +129,8 @@ See the "Finding references" section in the `review-criteria` skill for path-for Classify the overall review into exactly one level (none/low/medium/high) per the `review-criteria` skill. Domain context for this reviewer: - `none`: Code is already clean and well-structured — no simplification opportunities -- `low`: Minor opportunities (e.g., a handful of redundant or paraphrasing comments, one unused import) -- `medium`: Several meaningful simplification opportunities, including file-header-scale comment violations (tutorial headers, repeated conversation memorialization, broad library re-teaching) +- `low`: Minor opportunities (e.g., a handful of redundant or paraphrasing comments, one or two adjacent-test runs with shared-setup duplication, one unused import) +- `medium`: Several meaningful simplification opportunities, including file-header-scale comment violations (tutorial headers, repeated conversation memorialization, broad library re-teaching) or pervasive shared-setup duplication across a spec file - `high`: Pervasive unnecessary complexity indicating the code needs a simplification pass ## Output format From a0d20b1a24adcdc14bdf8d57ee56bb1327650fca Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 26 May 2026 19:16:50 -0700 Subject: [PATCH 5/6] agents|fix: Normalize test-structure audit checklist and wrap include Strip the test-structure-audit-checklist partial of content that duplicates its host: the diagnostic question and the "don't collapse distinct intents" carve-out both lived in the partial and in `### Diagnostic` / `### When N copies are right` of the host section. The partial now carries only the audit framing, the three refactor moves, and the closing failure-mode note, paralleling comment-audit-checklist.md's shape. Wrap the include in testing-conventions/SKILL.md in its own `### Audit before save` subheading so it reads as a distinct checkpoint, matching the structure comment-discipline.md uses around its own audit-checklist include. --- .../content/_partials/test-structure-audit-checklist.md | 6 +----- packages/agents/content/skills/testing-conventions/SKILL.md | 2 ++ 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/agents/content/_partials/test-structure-audit-checklist.md b/packages/agents/content/_partials/test-structure-audit-checklist.md index d5ccce28..cf573c18 100644 --- a/packages/agents/content/_partials/test-structure-audit-checklist.md +++ b/packages/agents/content/_partials/test-structure-audit-checklist.md @@ -1,11 +1,7 @@ Before saving a spec file, scan adjacent tests for shared-setup runs. -**The diagnostic.** For each run of three or more nearly-identical tests, ask: How many tokens does a reader have to diff to find what's actually different between them? If the answer is more than a handful, the signal is buried. Refactor. - - **Parameterize** with `it.each` when the variation is small and the test body is structurally identical. - **Extract a named helper** when the variation is bigger, when shared setup should disappear into a helper signature, or when assertion shapes differ per case. -- **Check the assertions, too.** When the rule is a count or predicate, specific-fixture-label assertions should not repeat per row. Assert specific data flow *once* in a focused test; assert structurally for the rest. - -Do not collapse distinct intents. The smell is shared *setup with one variable*, not shared *shape with different intents*. Three tests that read as genuinely distinct behavioral claims stay as three `it` blocks even if their bodies superficially resemble each other. +- **Check the assertions, too.** When the rule is a count or predicate, specific-fixture-label assertions should not repeat per row. Assert specific data flow _once_ in a focused test; assert structurally for the rest. Three or more `it` blocks with identical setup and a single varying input is the most common failure mode in interactive sessions. That material belongs behind a helper or a parameterized table, not in N copies of the same shape. diff --git a/packages/agents/content/skills/testing-conventions/SKILL.md b/packages/agents/content/skills/testing-conventions/SKILL.md index a8ecbc44..afe00318 100644 --- a/packages/agents/content/skills/testing-conventions/SKILL.md +++ b/packages/agents/content/skills/testing-conventions/SKILL.md @@ -87,6 +87,8 @@ Before writing a third test in the same `describe` block, scan the previous two: The smell is shared _setup with one variable_, not shared _shape with different intents_. Three tests that read as genuinely distinct behavioral claims should stay as three `it` blocks even if their bodies superficially resemble each other. Do not collapse distinct intents into a table. +### Audit before save + ## Additional patterns From e1ea3d6e8946a141ef41b08b75f44e1e2513e397 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 26 May 2026 19:16:57 -0700 Subject: [PATCH 6/6] agents|refactor: Split code-patterns Test structure subsection Split `### Test structure` in code-patterns/SKILL.md into two coherent subsections. The pointer to testing-conventions/SKILL.md now lives alone under `### Test structure` (the rules it references are about composition); the `describe()` argument-form and `__tests__/` sibling-placement bullets move to a new `### Test organization` subsection that matches their actual subject. --- packages/agents/content/skills/code-patterns/SKILL.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/agents/content/skills/code-patterns/SKILL.md b/packages/agents/content/skills/code-patterns/SKILL.md index 3151a8be..0c22d592 100644 --- a/packages/agents/content/skills/code-patterns/SKILL.md +++ b/packages/agents/content/skills/code-patterns/SKILL.md @@ -60,7 +60,9 @@ Follow the naming rules in [naming-conventions.md](../_data/naming-conventions.m ### Test structure -Test-structure rules (parameterization, helpers, signal-to-noise) live in [`testing-conventions/SKILL.md`](../testing-conventions/SKILL.md). +Rules live in [`testing-conventions/SKILL.md`](../testing-conventions/SKILL.md): parameterization, helpers, and signal-to-noise as the bar for both. + +### Test organization - **Use function/class reference as describe argument** - `describe(myFunction, ...)` instead of `describe('myFunction', ...)` - **Place tests in `__tests__` directory** as sibling to the file being tested