Test-structure guidance: Signal-to-noise as the bar for parameterization and helpers

[williamthorsen]

Problem

Test files frequently bury the salient input — the one prop, fixture, or value that actually varies between adjacent tests — in a wall of shared setup that's identical across N tests. The reader is forced to diff three or four nearly-identical render calls to find what's actually being tested. Specs become harder to scan, harder to maintain (one change has to be applied N times), and harder to reason about as a coherent rule.

This is structurally related to the comment-verbosity issue (#656) — both are signal-buried-in-noise failure modes — but the mechanism is different. The fix is not "delete things"; it's "factor the shared part out so the variation reads as variation."

Concrete examples (from the same recent commit)

A test file with this shape:

it('renders 2 chips and no overflow pill when 2 selections', () => {
  render(<StringHarness options={FRUITS} initialValue={['apple', 'banana']} />);
  ...
});

it('renders 2 chips plus a +1 pill when 3 selections', () => {
  render(<StringHarness options={FRUITS} initialValue={['apple', 'banana', 'cherry']} />);
  ...
});

it('renders 2 chips plus a +3 pill when 5 selections', () => {
  render(<StringHarness options={FRUITS} initialValue={['apple', 'banana', 'cherry', 'date', 'elderberry']} />);
  ...
});

The salient input — initialValue.length — is buried in a literal array of fixture labels. options={FRUITS} repeats across every test. Three tests covering the same rule (overflow-pill behavior) read like three unrelated scenarios.

Additionally, each test asserts on specific labels (screen.getByText('Apple'), screen.queryByText('Cherry')), which re-tests "the renderer puts data on screen" once per row rather than testing the rule (chip count + pill predicate).

Relevant considerations

• The bar should be signal-to-noise, not line count. Parameterized tables and helpers are usually comparable in length to the separated alternative — sometimes slightly shorter, sometimes slightly longer. The win is reader scannability, not byte count.
• Parameterization and helpers serve different cases.
  • it.each fits when the variation is small and the test body is structurally identical.
  • A named helper (e.g., renderWithThreshold(threshold, optionCount)) fits when the variation is bigger, when defaults should disappear into the helper signature, or when the assertion shape differs per case.
• Test the rule, not the data. When a renderer's behavior is "for input matching shape X, produce output matching shape Y," the spec should encode the rule (counts, predicates, structural claims), not enumerate inputs that happen to satisfy it. Specific labels should be asserted once, in a single focused test that proves data flows through correctly; the rest assert in terms of counts and predicates.
• it.each has a real failure-message cost. A failing it('renders 2 chips plus a +3 pill when 5 selections') is immediately diagnostic. A failing parameterized row requires reading Vitest's interpolated title. This cost is real but usually small — and it's the right trade when the alternative is N copies of the same setup.
• Don't over-parameterize distinct scenarios. Three tests that read as genuinely different behavioral claims should stay as three it blocks even if their bodies superficially resemble each other. The smell is shared setup with one variable, not shared shape with different intents.

Proposed solution

Add a concise test-structure section to the global agent guidance with three rules and one diagnostic.

Rules to add

1. Test bodies should be mostly variation and assertion, not setup. Any prop, fixture, or boilerplate that's 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 to add

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 — either parameterize the trio or extract a helper before continuing.

Mechanism

Same checkpoint shape as #656 — when wrapping up a spec file, scan adjacent tests for shared-setup runs. For each run of ≥3 nearly-identical tests, ask whether the shared part can become a helper or it.each table without making failure messages materially worse.

The code-simplification-reviewer subagent's Scope is extended to flag the same patterns — adjacent tests with near-identical setup, specific-label assertions repeated per row when the rule is structural, and helpers whose parameters the test bodies still re-specify. This mirrors how comment-discipline violations were added to the reviewer's scope in #659.

Acceptance

• Test-structure section added to shared agent guidance covering the three rules and the diagnostic.
• A short note added to the interactive-coding flow that triggers the adjacent-test diff check before declaring a spec file complete.
• code-simplification-reviewer scope extended to flag test-structure violations.
• Existing one-line it.each mention in code-patterns/SKILL.md either dropped or replaced with a pointer to testing-conventions/SKILL.md (single source of truth for the rule).
• Verified by re-running a comparable spec-writing task and observing that adjacent tests with identical setup get factored (helper or it.each) on first pass rather than left as N copies.

[williamthorsen]

Adding a tighter restatement of the principle that surfaced while planning the worked-example refactor:

Every test body should be mostly the variation and the assertion, with no shared boilerplate.

If a test body contains setup that's identical across N adjacent tests, that setup is noise and belongs in a helper or it.each table. The two ingredients that must be in the body are (a) what's different about this case and (b) what's being asserted. Everything else is a candidate for extraction.

This is a more compact version of rule 1 in the issue body, and the form most useful as a one-line checkpoint.