mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-11 08:20:53 +00:00
87fab87d84
## Summary Adds `.claude/skills/reviewing-unit-tests/SKILL.md`, a project-scoped agent skill that gives reviewing agents a concrete rubric for evaluating Vitest unit-test diffs. ## Changes - **What**: New skill at `.claude/skills/reviewing-unit-tests/SKILL.md`. Mirrors the structural template of the peer skill `.claude/skills/writing-playwright-tests/SKILL.md` (frontmatter shape, table-driven sections, terse prescriptive prose). Description triggers on PR review of `*.test.ts` files. Codifies: alias-by-renaming detection, `vi.mocked()` scope rule, unnecessary-cast detection, divergence checks against `docs/testing/unit-testing.md` / `store-testing.md` / `component-testing.md` / `vitest-patterns.md`, redundant `mockClear()` flag, `vue-i18n`-must-not-be-mocked rule (with `testI18n` reference), bugfix regression-validity check, leading-close framing flag, testing-library requirement for new component tests. ## Review Focus - **Retrospective validation against PR #11737** (head SHA `4573d62450fd12ac6f06e5e491f8af84ccbd27de`, file `src/platform/assets/composables/useMediaAssetActions.test.ts`). Mentally running the skill's checklist against that diff flags: 1. ✅ `getToastAddMock` / `getI18nTMock` module-level helpers — caught by the "Renaming ≠ Restructuring" section + Mocking Smells row "Module-level helper functions wrapping mocked composable returns". 2. ✅ `as ReturnType<typeof vi.fn>` casts in those helpers — caught by the "`vi.mocked()` Scope" section + Mocking Smells row on stray casts. 3. ✅ `vi.mock('vue-i18n', ...)` — caught by Mocking Smells row directing to mount real `createI18n` per `docs/testing/vitest-patterns.md` and the shared `testI18n` in `src/components/searchbox/v2/__test__/testUtils.ts`. 4. ✅ `vi.mock('primevue/usetoast', ...)` — explicitly **not** flagged. The "Distinguish" paragraph after the Mocking Smells table marks trivially-shaped third-party hooks as acceptable to mock; "Don't Mock What You Don't Own" applies to behavior-rich APIs, not single-method composables. - **Reference style**: links to `docs/testing/*.md` and `docs/guidance/typescript.md` instead of restating their content, per the FE-511 acceptance criterion. - **Open question**: scope. The repo-local skill is the FE-511 deliverable; equivalent prescriptive rules already live in the user's global `~/.config/amp/AGENTS.md` "Code Review Rigor" section as a separate artifact. No duplication intended — the skill is the project-scoped surface a reviewing agent loads when entering this repo. Closes FE-511 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11777-docs-add-reviewing-unit-tests-skill-3526d73d365081848759e6a8fab942f0) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com>