Reviewing Unit Tests

Over-Mocking and Replicated Logic

• Count vi.mock, vi.fn, vi.spyOn, and hand-built fakes per file. Above three, ask what the test still proves about owned code. The count is a signal, not a verdict. Boundary doubles around exercised owned code are fine. Request changes when no owned behavior runs or a double replaces owned code.
• Do not count shared automocks: a factory-less vi.mock(import('…')) that activates a __mocks__/ file, plus vi.mocked(...) configuration of it, per docs/guidance/vitest.md "Shared manual mocks". The exemption covers the count only. Automocking the module under test is still a finding.
• A hand-written vi.mock factory for a module that already has a __mocks__/ file is a duplicated fixture.
• Deletion test: imagine deleting the production wiring. If every test still passes, nothing covers it.
• A test that re-implements the code it covers passes on bugs that both copies share. Request the real module plus a double at its true boundary (network, clock, reportError).
• Name the Desiderata property the test gives up, usually predictive, behavioral, or structure-insensitive. See docs/guidance/testing-principles.md "Judge every test by Beck's Test Desiderata".

Alias-by-Renaming

// Before
const mockAdd = vi.hoisted(() => vi.fn())

// After: same indirection, new name
function getToastAddMock() {
  return useToast().add
}

If the wrapper only renames or relays a mocked value, fail it. Inline the lookup at the call site or fetch the singleton mock via the documented pattern.

vi.mocked() Scope

• Flag casts whenever vi.mocked() would narrow correctly.
• Do not add vi.mocked() around assertion-only references just for style.

Reset Hygiene

• vite.config.mts sets mockReset, restoreMocks, unstubEnvs, and unstubGlobals. Flag manual resets, restores, and global save/restore that duplicate them.
• Flag per-mock mockClear() or mockReset() when vi.clearAllMocks() or vi.resetAllMocks() already runs in the relevant hook chain.
• Review for redundancy or broken state management. Do not bikeshed clearAllMocks vs resetAllMocks unless behavior depends on it.

Third-Party Seams

• Distinguish trivial hooks from behavior-rich APIs.
• Mocking single-method third-party hooks like @vueuse/core's useClipboard is usually acceptable.
• That exception does not justify mocking behavior-rich third-party modules.

vue-i18n

• Never mock vue-i18n in component tests.
• Use real createI18n per docs/testing/vitest-patterns.md and the shared testI18n setup.

Test-Body Rules

Bugfix Regression Proof

For fix: PRs or bugfix diffs:

1. Identify the production change that fixes the bug.
2. Verify the new test fails on pre-fix code, or ask the author to show it.
3. If the test passes on broken code, request changes.

A regression test that never proves red does not pin the bug.

Review Output Rules

• State verdict before procedural questions.
• Do not lead with approval language like LGTM, just one nit or approve and move on?.
• Name the failure mode directly: alias-by-renaming, unnecessary cast, mocked i18n, mock-only assertion, unproven regression, duplicated fixture, over-mocking, replicated logic, hand-rolled runner primitive.
• Link the authoritative doc section in the review comment.
• If an explicit repo rule, lint rule, or authoritative doc note is violated, do not downgrade it to "minor deviation" or "nit".

Quick Reference

Key Files to Read