From 95ab4922ff76616987755fcebec9540a040ae70a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E4=B8=80=E4=B9=8B?= Date: Tue, 21 Jul 2026 10:56:00 +0800 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9C=85=20=E5=AE=8C=E5=96=84=E6=B5=8B?= =?UTF-8?q?=E8=AF=95=E8=AE=BE=E8=AE=A1=E8=A7=84=E8=8C=83=E5=B9=B6=E6=B8=85?= =?UTF-8?q?=E7=90=86=E4=BD=8E=E4=BB=B7=E5=80=BC=E7=94=A8=E4=BE=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 2 +- docs/DOC-MAINTENANCE.md | 2 +- docs/README.md | 2 +- docs/develop.md | 11 +- docs/references/develop-testing.md | 198 +++++++++++++++--- eslint.config.mjs | 25 +++ scripts/git-staged-snapshot.test.mjs | 80 +++---- src/pages/components/NameAvatar.test.tsx | 11 +- src/pages/components/ui/empty-state.test.tsx | 15 +- .../Agent/components/AgentEmptyState.test.tsx | 20 -- .../options/routes/Setting/index.test.tsx | 56 ----- src/pages/options/routes/Tools/index.test.tsx | 64 ------ tests/vitestSetupImports.test.ts | 13 -- 13 files changed, 253 insertions(+), 246 deletions(-) delete mode 100644 src/pages/options/routes/Agent/components/AgentEmptyState.test.tsx delete mode 100644 src/pages/options/routes/Setting/index.test.tsx delete mode 100644 src/pages/options/routes/Tools/index.test.tsx delete mode 100644 tests/vitestSetupImports.test.ts diff --git a/AGENTS.md b/AGENTS.md index 17091d115..26fde137d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,7 +54,7 @@ isn't universal, that's called out in the item itself. - **Fix root causes, not symptoms — refactor over patch.** No `as any` / `// @ts-ignore` / try-catch swallow / defensive skips to make errors disappear (宁愿重构也不要打补丁). If a test fails, fix the code, not the test — the narrow exceptions (a wrong test contract; a test that never carried value) are in [`docs/references/develop-testing.md`](docs/references/develop-testing.md#writing-meaningful-tests-what-to-clean-up--not-write). - **Confirm before you fix.** Before touching a reported bug, reproduce it and confirm it actually exists — never fix from assumption. Capture the reproduction, then fix, **in that order** (确定 bug 存在 → 写测试或记录验证证据 → 修复); how to reproduce and what counts as capture are in [`docs/verification.md`](docs/verification.md) and the TDD entry below. -- **TDD/BDD first, for changes that alter observable behavior.** Write failing tests **before** implementing new or changed behavior, using BDD-style Chinese `describe`/`it` titles. Two narrow exceptions — neither a blanket file/task category — are in [`docs/references/develop-testing.md`](docs/references/develop-testing.md#when-tdd-doesnt-apply). (Runner, mocks, and how to run tests are in `docs/develop.md`.) +- **TDD/BDD first, for changes that alter observable behavior.** Write failing tests **before** implementing new or changed behavior, using BDD-style `describe`/`it` titles (Chinese or English). Two narrow exceptions — neither a blanket file/task category — are in [`docs/references/develop-testing.md`](docs/references/develop-testing.md#when-tdd-doesnt-apply). (Runner, mocks, and how to run tests are in `docs/develop.md`.) - **SOLID, high cohesion & low coupling — applied to the existing extension points.** Persistence is a small backend taxonomy (`Repo` / `DAO` / `OPFSRepo` / a few custom repos), not one pattern to default to — pick by matching an existing entity with the same needs; see [`docs/references/architecture-data.md`](docs/references/architecture-data.md#adding-an-entity). For messages, use `Group.on(...)`. Not every service takes the same constructor shape — context services vs. the Agent subsystem differ; see [`docs/references/architecture-services.md`](docs/references/architecture-services.md#adding-a-service). Depend on narrow interfaces (`IMessageQueue`, not `MessageQueue`). - **Direct replacement over adapter sandwiches.** When swapping a backend/library, replace in place — no `interface Foo + LegacyImpl + NewImpl` unless both must coexist at runtime. - **Scope discipline — stay in your lane.** Bug fix ≠ cleanup PR. Touch only the files the task requires; leave unrelated files untouched (不要动和任务不相干的文件). Don't add helpers, abstractions, validation, or backwards-compat shims you don't need today. Three similar lines beats a premature abstraction. Don't remove or narrow currently supported behavior just to simplify a fix — only do so when the task or an already-verified contract explicitly calls for that change. diff --git a/docs/DOC-MAINTENANCE.md b/docs/DOC-MAINTENANCE.md index 88c39fa4d..c66c277f7 100644 --- a/docs/DOC-MAINTENANCE.md +++ b/docs/DOC-MAINTENANCE.md @@ -89,7 +89,7 @@ of sanitization patterns can otherwise look like matches — so don't rely on a | Doc | Owns | | --- | --- | | [`../AGENTS.md`](../AGENTS.md) | Engineering principles + architecture quick-map. Single source of truth; `CLAUDE.md` only `@import`s it. | -| [`develop.md`](./develop.md) | The concrete "how": commands, structure, style, i18n, commit/PR; testing mechanics split to [`references/develop-testing.md`](./references/develop-testing.md). | +| [`develop.md`](./develop.md) | The concrete "how": commands, structure, style, i18n, commit/PR; testing (design, cleanup policy, mechanics) split to [`references/develop-testing.md`](./references/develop-testing.md). | | [`pull-request.md`](./pull-request.md) | Detailed PR description structure and guidance for agents and contributors; the human-facing template remains lightweight. | | [`design.md`](./design.md) | The design system: theme mechanism, shadcn component selection, new-page recipe; tokens split to [`references/design-tokens.md`](./references/design-tokens.md), component palette to [`references/design-components.md`](./references/design-components.md), layout/motion/state/a11y patterns to [`references/design-patterns.md`](./references/design-patterns.md). | | [`verification.md`](./verification.md) | Lightweight end-to-end functional verification — throwaway scratch scripts driving the real built extension; report template split to [`references/verification-report-template.md`](./references/verification-report-template.md), debugging FAQ to [`references/verification-debugging.md`](./references/verification-debugging.md). | diff --git a/docs/README.md b/docs/README.md index 13fc84aa4..8db124fa5 100644 --- a/docs/README.md +++ b/docs/README.md @@ -7,7 +7,7 @@ | 文档 | 说明 | | --- | --- | | [`../AGENTS.md`](../AGENTS.md) | 工程原则、架构速览、AI/贡献者约定的单一信息源 —— 但仅相对 `CLAUDE.md`(其仅导入它)成立;`.github/copilot-instructions.md` 是 Copilot 的独立入口,与本文件共享的事实需在两边都改动时做一致性核对(parity review)。 | -| [`develop.md`](./develop.md) | 开发规范:命令、目录结构、编码风格、UI/主题、i18n、提交/PR 流程;测试机制(含 Vitest 性能)拆到 [`references/develop-testing.md`](./references/develop-testing.md)。**写代码前先读。** | +| [`develop.md`](./develop.md) | 开发规范:命令、目录结构、编码风格、UI/主题、i18n、提交/PR 流程;测试设计/清理口径与运行机制(含 Vitest 性能)拆到 [`references/develop-testing.md`](./references/develop-testing.md)。**写代码前先读。** | | [`pull-request.md`](./pull-request.md) | PR 描述指南:代理与贡献者使用的详细章节、按变更类型取舍规则、验证与审查信息要求。 | | [`design.md`](./design.md) | 设计系统参考:主题机制、shadcn 组件选型、新建页面配方总览;令牌完整值拆到 [`references/design-tokens.md`](./references/design-tokens.md),组件清单拆到 [`references/design-components.md`](./references/design-components.md),布局/响应式/动效/状态/无障碍范式拆到 [`references/design-patterns.md`](./references/design-patterns.md)。**做页面/对话框/区块前先读。** | | [`verification.md`](./verification.md) | 功能验证指南:用一次性 scratch 脚本驱动真实扩展做端到端验证(不跑全量 E2E、不加永久用例);报告模板拆到 [`references/verification-report-template.md`](./references/verification-report-template.md),调试 FAQ 拆到 [`references/verification-debugging.md`](./references/verification-debugging.md)。**验证改动是否真正跑通时读。** | diff --git a/docs/develop.md b/docs/develop.md index 691760815..065debd84 100644 --- a/docs/develop.md +++ b/docs/develop.md @@ -69,12 +69,15 @@ it as settled fact: (`bg-white`, `text-gray-500`, `dark:bg-gray-800`, `bg-[#fff]`); use design tokens (`bg-background`/ `text-foreground`/…) so light & dark both work. -Two conventions are enforced via built-in rules in `eslint.config.mjs`: `no-restricted-imports` bans +Three conventions are enforced via built-in rules in `eslint.config.mjs`: `no-restricted-imports` bans `@radix-ui/react-*` single packages (use the merged `radix-ui`) and the `sonner` `toast` export (use `notify`); -`no-restricted-syntax` bans `forwardRef` across `src/pages/**` (use React 19 `function` + ref-prop). +`no-restricted-syntax` bans `forwardRef` across `src/pages/**` (use React 19 `function` + ref-prop); and a +file-scoped `no-restricted-imports` on `tests/vitest.setup.ts` bans `./utils` / `@App/app/service*` / +`@App/pages/store*` so global test setup stays lightweight (as a per-file rule replacement it also drops the +sonner/radix restriction there — the file imports neither). `eslint-rules/harness.test.mjs` covers exactly four of these: `no-i18n-default-value`, `no-raw-color-classname`, the `radix-ui` pattern of `no-restricted-imports`, and `no-restricted-syntax` — not `require-last-error-check`, -and not the `sonner` pattern of `no-restricted-imports`. +not the `sonner` pattern of `no-restricted-imports`, and not the `tests/vitest.setup.ts` scope. `src/pages/components/ui/toast.ts` has an override that turns `no-restricted-imports` **entirely off** for that one file — not just the `sonner` half of it. Only the `sonner` exception is intentional: this is the one place @@ -131,7 +134,7 @@ React 19 + shadcn/ui (Radix UI primitives, "new-york" style) + Tailwind CSS v4 + This project uses Vitest for unit tests and Playwright for end-to-end tests. -> Mechanics, meaningful-test guidance, and Vitest performance hygiene live in [testing.md](./references/develop-testing.md). To verify a change end-to-end without growing the suite, see [verification.md](./verification.md). +> Test design, meaningful-test & cleanup guidance, run mechanics, and Vitest performance hygiene live in [testing.md](./references/develop-testing.md). To verify a change end-to-end without growing the suite, see [verification.md](./verification.md). ## i18n diff --git a/docs/references/develop-testing.md b/docs/references/develop-testing.md index 034fd47a5..eec5ee913 100644 --- a/docs/references/develop-testing.md +++ b/docs/references/develop-testing.md @@ -3,33 +3,110 @@ > The **TDD/BDD-first principle** (write failing tests before implementation; fix code not tests) lives in > [AGENTS.md § Engineering Principles](../../AGENTS.md#engineering-principles). This section is the mechanics. -Vitest + happy-dom. Per-test budgets live in `vitest.config.ts` per project: non-UI projects (`fast`, -`isolated`) use 340ms; the `ui` project (`src/pages/**/*.test.{ts,tsx}` — React renders, including -`renderHook` tests in `.ts` files) uses 850ms because a render + interaction case genuinely costs 100–200ms -solo under coverage and worker parallelism multiplies that (fake-timer countdown cases have been observed at -~630ms under full local load). Don't pass `--test-timeout` on the CLI — it would override every project's -budget at once. Chrome APIs are mocked via -`@Packages/chrome-extension-mock` (`tests/vitest.setup.ts`). `MockMessage` is available for message-system tests. -`happy-dom` is patched via `patches/` (see `pnpm-workspace.yaml` `patchedDependencies`) to build its -invalid-selector `DOMException` lazily — the upstream eager construction captures a deep stack on every -`matches()`/`querySelector()` call, which is measurably slower at TSX-suite scale. No specific percentage is -tracked here since it isn't tied to a reproducible command/environment; if you need a number, measure -before/after this patch in the same environment using the JSON-report method below rather than trusting a -historical figure. +This guide owns how contributors design, write, review, clean up, and run automated tests. A test is not useful +merely because it raises coverage: it must protect an observable contract, fail for a relevant regression, and +cost less to understand and maintain than the confidence it provides. -- Co-locate `*.test.ts`/`*.test.tsx` next to source (or place in `tests`). -- BDD-style Chinese `describe`/`it` titles. Use `describe.concurrent()` / `it.concurrent()` where independent. -- Single file: `pnpm test -- --run path/to/file.test.ts`. -- Playwright tests are `*.spec.ts` files in `e2e`; they run with one worker and retain failure artifacts. Run targeted tests while iterating, then run `pnpm run lint` plus the relevant full suite before a PR. +## Designing a test before writing it + +Start from behavior, not from the current implementation. Before writing assertions, state four things: + +1. **Contract** — what callers or users can observe. +2. **Trigger** — the input, event, state, or sequence that exercises the contract. +3. **Outcome** — the returned value, rendered state, persisted data, emitted message, or external call that must + result. +4. **Regression** — a plausible wrong implementation that this test would reject. + +If the regression cannot be named, the proposed test is probably asserting an implementation detail or a +tautology. For a reported bug, first reproduce and capture the failure as required by +[verification.md](../verification.md); then make the smallest test that fails for that confirmed cause. + +Choose the narrowest test boundary that still observes the real contract: + +- Use a pure unit test for parsing, mapping, validation, selection, and state-transition logic. +- Render a focused component when conditional UI, accessibility derivation, interaction, or variant-to-token + mapping is the contract. +- Use a service or repository test when persistence, messages, retries, ordering, or lifecycle behavior crosses + an object boundary. +- Use an integration or E2E test when the failure depends on a real browser API, extension context, build entry, + worker boundary, or several components being wired together. Do not force such work into a heavily mocked unit + test merely to make it cheap. +- Use the throwaway workflow in [verification.md](../verification.md) when permanent automation is genuinely + infeasible or would cost more than the regression risk warrants. + +### Cover the behavior space deliberately + +A behavior-changing test set normally starts with the **normal case** and then adds the boundaries and failure +paths that can change the outcome. Do not stop after proving one happy-path example, but do not mechanically +enumerate inputs that all execute the same branch either. + +For each changed contract, consider: + +- **Normal case** — representative valid input completes successfully and produces the intended observable + result. +- **Boundary cases** — empty and single-item inputs; first/last item; exact size, time, or count limit; values just + below and above a threshold; missing optional fields; duplicate items; and Unicode or special paths when the + code treats them differently. +- **Invalid and failure cases** — malformed input, rejected dependency, permission denial, timeout, cancellation, + partial data, or unavailable browser capability. Assert whether the contract rejects, reports, retries, rolls + back, or preserves prior state. +- **State transitions** — before/after state, repeated calls, idempotency, cleanup, unsubscribe/dispose behavior, + and whether stale async work can overwrite newer work. +- **Ordering and concurrency** — out-of-order completion, overlapping operations, deduplication, and exactly-once + effects when the production code promises them. +- **Compatibility/security boundaries** — legacy accepted forms, cross-browser branches, untrusted URLs or paths, + access scope, and maximum payload limits when they are part of the contract. + +Select cases by distinct equivalence classes and branches. For example, if `value === limit`, `value < limit`, and +`value > limit` follow three different outcomes, cover all three. If ten ordinary strings take the same path, +one representative string is normally enough. An empty array is worth its own test only when emptiness changes +behavior; it is redundant when it follows the exact same branch and assertion as a non-empty array. + +For bug fixes, include a regression case that matches the confirmed failure conditions closely enough that +reintroducing the cause makes it fail. Also keep a normal-case assertion when the fix could accidentally narrow +existing supported behavior. + +### Assert outcomes, not incidental structure + +Prefer assertions at the public boundary: + +- Assert returned domain values, persisted records, visible state, accessibility attributes, messages, or the + minimum necessary collaborator call. +- Assert collaborator calls when the call itself is the contract, such as “do not write before approval” or + “publish exactly once”; do not assert every internal call made along the way. +- Prefer exact assertions for structured output. Use broad `toContain`/truthiness assertions only when the + omitted details are intentionally outside the contract. +- A test name must describe what the body actually triggers and observes. Use BDD-style `describe`/`it` titles — + Chinese and English are both fine; avoid vague names such as `works`, `test1`, or a bug label without the + behavior. +- One test may contain several related assertions for one behavior. Do not split every property into a separate + setup-heavy test, and do not combine unrelated contracts into one scenario. -### When TDD doesn't apply +### Mocks and fixtures + +Mock at external or expensive boundaries, not at every internal function. A useful mock makes the test +deterministic while preserving the production path under test. + +- Prefer the repository's shared mocks — `@Packages/chrome-extension-mock` (Chrome APIs), `MockMessage` from + `@Packages/message/mock_message`, `@Tests/mocks/pageStores.ts` (page stores) — over hand-built partial objects. +- Give a mock only the behavior needed by the scenario, but keep it structurally compatible with the narrow + interface consumed by the subject. +- Do not render a page with many live child sections and then maintain unrelated client mocks just to assert a + static button count. Test the category generator, focused section behavior, or a real integration boundary. +- Assert how our code transforms, routes, persists, or reacts to what a mock returns — never that the mock + returned what it was configured to return (that is testing the mock, a no-value category below). +- Fixtures should be small enough that the meaningful difference is visible. Builders are useful when defaults + are stable and scenarios override only relevant fields; avoid builders that hide the input responsible for a + regression. + +## When TDD doesn't apply Two exceptions to the TDD/BDD-first principle, neither a blanket file/task category — write a failing test for everything else: - **Genuinely behavior-preserving work** — refactors, type cleanup, dead-code removal, mechanical renames, or a config/dependency change confirmed not to alter behavior. Verify it instead of testing it. - **Automated coverage is genuinely infeasible** — a pure visual/animation tweak, a bug reproducible only in a specific browser version or extension lifecycle stage, a copy/translation wording change, platform behavior that can't be automated reliably. Verify it manually and record the evidence instead of committing a pass-through or low-value test just to satisfy the rule — [`../verification.md`](../verification.md)'s scratch-extension workflow when the change needs the built extension or browser APIs, a simpler noted check otherwise. -### Writing meaningful tests (what to clean up / not write) +## Writing meaningful tests (what to clean up / not write) **Two distinct situations — don't conflate them.** A test that fails because *its own asserted contract* is wrong (a stale fixture, an assertion that was incorrect from the start, a contract that legitimately changed) — fix the test and say why. A test that never carried value regardless of pass/fail (see below) — clean it up independent of whether it's currently failing. Neither is license to weaken a valid regression test just to make CI pass. @@ -41,7 +118,7 @@ A test earns its place by exercising **our own logic** and failing on a real reg - **Pure pass-through render** — `render()` that only asserts `x` shows up, with no branching / variant mapping / derived logic in the component. - **Testing the mock or framework, not our code** — configuring a `vi.fn()` then asserting it returned what it was fed; asserting a third-party lib's or the JS language's own semantics. - **Mislabeled** — the test name claims a behavior the body never triggers (e.g. claims to test abort but never calls abort). Worse than no test: it gives false confidence. -- **File-content assertion that belongs in a lint rule** — reading a source file and grepping its text for a token/string is a mechanical convention; express it as an ESLint rule, not a unit test. +- **File-content assertion that belongs in a lint rule** — reading a source file and grepping its text for a token/string is a mechanical convention; express it as an ESLint rule or a structural-harness check (see [develop.md § ESLint custom rules](../develop.md#eslint-custom-rules)), and land the verified replacement guard **before** deleting the Vitest test — e.g. `tests/vitest.setup.ts`'s lightweight-setup constraint is a file-scoped `no-restricted-imports` in `eslint.config.mjs`. Conversely, keep these — they look thin but carry real value: @@ -50,12 +127,85 @@ Conversely, keep these — they look thin but carry real value: - `instanceof` / `name` guards on custom `Error` subclasses, security-blocklist completeness, and similar regression guards. - The **only** coverage of a component / sub-component — deleting it removes coverage, not noise. -> **Verify each against the source before deleting.** Many "looks meaningless" tests actually exercise a real branch; judging in bulk from a scan over-flags heavily. Confirm the behavior genuinely exists / is covered elsewhere before removing anything. +### Cleaning up tests safely + +Tests are production dependencies: stale or meaningless tests should be removed, but a failing or slow test is +not automatically meaningless. Classify the problem before editing it: + +- **Production regression** — the asserted contract is still valid and production code violates it. Fix the + production code. +- **Wrong or obsolete contract** — requirements legitimately changed, or the assertion was incorrect. Update or + replace the test and record the contract change. +- **Flaky test** — timing, leaked global state, nondeterministic ordering, or an uncontrolled dependency changes + the result. Reproduce the flake and fix its cause; do not add retries or a large timeout without evidence. +- **Misclassified integration work** — real browser/process/I/O work exceeds a pure-unit budget under CI + contention. Put it in the appropriate project or give the specific case a measured budget; do not delete the + behavior or relax every test globally. +- **No-value test** — it matches one of the categories above and removing it loses no distinct regression + detection. Delete it rather than preserving it for coverage numbers. +- **Valuable constraint in the wrong mechanism** — a file-content assertion protecting a real rule: migrate it + per the category above, replacement guard first. + +Before deleting or consolidating a test, verify each against the source, one by one — judging in bulk from a +scan over-flags heavily, and many "looks meaningless" tests actually exercise a real branch: + +1. Read the production path it claims to cover; do not judge from its name or line count. +2. Search for the same contract in nearby unit tests, caller tests, integration tests, E2E tests, lint rules, and + structural harnesses. +3. Identify the mutation/regression the test rejects. If another test would fail for the same regression, show + why; similar setup or output alone does not prove duplication. +4. Check whether the test is the only coverage of a conditional, error path, security boundary, lifecycle event, + browser variant, or historical regression. +5. Delete or consolidate only the redundant signal, not assertions that cover distinct branches. Parameterize + repeated setup when it makes the behavior matrix clearer. +6. Run the focused remaining tests, then the relevant full suite. For timing or concurrency issues, reproduce + the CI combination — see [Vitest Performance Hygiene](#vitest-performance-hygiene). + +## Test author & review checklist + +One checklist, used at two moments: self-check before handing off a behavior-changing implementation, and review +of any new or changed test. + +- [ ] Reproduced the reported bug or wrote the failing behavior test before production changes, and confirmed it + fails for the intended regression — a plausible broken implementation would trip it, not merely missing + mocks or unrelated setup. +- [ ] Covered a representative normal case plus each outcome-changing boundary, invalid input, and failure path — + every case a distinct branch or equivalence class, not another sample of the same path. +- [ ] Covered ordering, cleanup, cancellation, or repeated-call behavior when the contract involves state or + asynchronous work; the test is deterministic, isolated, and cleaned up after itself. +- [ ] Assertions observe our contract at the public boundary — not React/Vitest/a third-party library, a + configured mock, or incidental structure — and the title matches the trigger and assertion. +- [ ] Used the narrowest realistic boundary, with the fewest and most stable mocks available. +- [ ] Removed no-value tests (per the categories above) instead of keeping them for coverage numbers; source-text + rules are enforced by lint or a structural harness, not a unit test. +- [ ] Ran the focused test, `pnpm run lint`, and the relevant full suite; recorded only commands actually run. + +## Running tests + +Vitest + happy-dom. Per-test budgets live in `vitest.config.ts` per project: non-UI projects (`fast`, +`isolated`) use 340ms; the `ui` project (`src/pages/**/*.test.{ts,tsx}` — React renders, including +`renderHook` tests in `.ts` files) uses 850ms because a render + interaction case genuinely costs 100–200ms +solo under coverage and worker parallelism multiplies that (fake-timer countdown cases have been observed at +~630ms under full local load). Don't pass `--test-timeout` on the CLI — it would override every project's +budget at once. Chrome APIs are mocked via +`@Packages/chrome-extension-mock` (`tests/vitest.setup.ts`). `MockMessage` is available for message-system tests. +`happy-dom` is patched via `patches/` (see `pnpm-workspace.yaml` `patchedDependencies`) to build its +invalid-selector `DOMException` lazily — the upstream eager construction captures a deep stack on every +`matches()`/`querySelector()` call, which is measurably slower at TSX-suite scale. No specific percentage is +tracked here since it isn't tied to a reproducible command/environment; if you need a number, measure +before/after this patch in the same environment using the JSON-report method below rather than trusting a +historical figure. + +- Co-locate `*.test.ts`/`*.test.tsx` next to source (or place in `tests`). +- Use `describe.concurrent()` / `it.concurrent()` where independent. +- Single file: `pnpm test -- --run path/to/file.test.ts`. +- Playwright tests are `*.spec.ts` files in `e2e`; they run with one worker and retain failure artifacts. Run targeted tests while iterating, then run `pnpm run lint` plus the relevant full suite before a PR. -### Vitest Performance Hygiene +## Vitest Performance Hygiene - Keep `tests/vitest.setup.ts` lightweight. Shared setup should only install global browser/chrome mocks; heavier - feature helpers belong in opt-in test utilities. + feature helpers belong in opt-in test utilities. Enforced by a file-scoped `no-restricted-imports` in + `eslint.config.mjs`. - For files that use one fixed UI language, prefer `initTestLanguage()` from `tests/initTestLanguage.ts` in `beforeAll` over repeated `initLanguage()` calls inside every test. Tests that intentionally switch languages should keep explicit language setup. @@ -114,5 +264,5 @@ jq -r '.testResults[] | .name as $file | .assertionResults[] | [.duration, $file ``` > To **verify a change works end-to-end without growing the suite** — drive the real built extension with a -> throwaway scratch script — see [`VERIFICATION.md`](../verification.md). That is lightweight verification, not +> throwaway scratch script — see [`verification.md`](../verification.md). That is lightweight verification, not > the committed test suite owned by this section. diff --git a/eslint.config.mjs b/eslint.config.mjs index 78024e366..215f0a05a 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -97,6 +97,31 @@ export default [ files: ["src/pages/components/ui/toast.ts"], rules: { "no-restricted-imports": "off" }, }, + { + // 全局测试 setup 每个测试文件都要加载，引入重型模块会拖慢整个套件 + // （见 docs/references/develop-testing.md § Vitest Performance Hygiene）。 + files: ["tests/vitest.setup.ts"], + rules: { + "no-restricted-imports": [ + "error", + { + patterns: [ + { + group: [ + "./utils", + "./utils/*", + "@App/app/service", + "@App/app/service/*", + "@App/pages/store", + "@App/pages/store/*", + ], + message: "全局 setup 只安装浏览器/chrome mock，重型 helper 放到按需 import 的测试工具里。", + }, + ], + }, + ], + }, + }, { // new-ui 页面禁止 className 写原始颜色，必须走设计令牌以适配亮/暗主题。 files: ["src/pages/**/*.tsx"], diff --git a/scripts/git-staged-snapshot.test.mjs b/scripts/git-staged-snapshot.test.mjs index 8e9c941eb..591476103 100644 --- a/scripts/git-staged-snapshot.test.mjs +++ b/scripts/git-staged-snapshot.test.mjs @@ -24,8 +24,6 @@ function makeTempRepo() { tmpDirs.push(dir); mkdirSync(dir, { recursive: true }); git(dir, ["init", "-q"]); - git(dir, ["config", "user.email", "test@example.com"]); - git(dir, ["config", "user.name", "Test"]); return dir; } @@ -36,72 +34,48 @@ afterEach(() => { }); describe("materializeStagedSnapshot", () => { - it("暂存坏内容、工作区改回好内容时，快照应反映暂存区（坏）内容", () => { + it("工作区与暂存区内容不同时应物化暂存区内容", () => { const repo = makeTempRepo(); - writeFileSync(path.join(repo, "a.json"), JSON.stringify({ good: true })); - git(repo, ["add", "a.json"]); - git(repo, ["commit", "-q", "-m", "init"]); + const source = path.join(repo, "a.json"); - // 暂存"坏"内容 - writeFileSync(path.join(repo, "a.json"), JSON.stringify({ bad: true })); + writeFileSync(source, JSON.stringify({ source: "index" })); git(repo, ["add", "a.json"]); - // 工作区恢复为"好"内容（不影响索引） - writeFileSync(path.join(repo, "a.json"), JSON.stringify({ good: true })); + writeFileSync(source, JSON.stringify({ source: "working-tree" })); const dest = path.join(repo, "..", `snapshot-${Math.random().toString(36).slice(2)}`); tmpDirs.push(dest); materializeStagedSnapshot(repo, dest); const snapshotContent = JSON.parse(readFileSync(path.join(dest, "a.json"), "utf8")); - expect(snapshotContent).toEqual({ bad: true }); - }); - - it("未暂存的工作区改动不应出现在快照中", () => { - const repo = makeTempRepo(); - writeFileSync(path.join(repo, "a.json"), JSON.stringify({ v: 1 })); - git(repo, ["add", "a.json"]); - git(repo, ["commit", "-q", "-m", "init"]); - - // 只改工作区，不 git add - writeFileSync(path.join(repo, "a.json"), JSON.stringify({ v: 2 })); - - const dest = path.join(repo, "..", `snapshot-${Math.random().toString(36).slice(2)}`); - tmpDirs.push(dest); - materializeStagedSnapshot(repo, dest); - - const snapshotContent = JSON.parse(readFileSync(path.join(dest, "a.json"), "utf8")); - expect(snapshotContent).toEqual({ v: 1 }); + expect(snapshotContent).toEqual({ source: "index" }); }); // 与 check-i18n.mjs 同源的入口守卫问题：脚本路径含空格 / 非 ASCII 字符时 CLI 分支不执行， // 快照目录留空。pre-commit 里它与 check-i18n 用 `&&` 串联，两者一起静默放行坏提交。 describe("CLI 入口守卫（脚本路径含空格 / 非 ASCII 字符）", () => { - // 真实 spawn node 子进程（实测 200ms+），不适用 vitest.config.ts 给单元测试定的 340ms 预算。 + // 真实启动 Node 和 Git 子进程，不适用 vitest.config.ts 给进程内单元测试定的 340ms 预算。 const CLI_TIMEOUT = 15_000; - for (const dirName of ["with space", "中文目录"]) { - it( - `目录名 "${dirName}" 下仍应真正物化暂存快照`, - () => { - const repo = makeTempRepo(); - writeFileSync(path.join(repo, "a.json"), JSON.stringify({ v: 1 })); - git(repo, ["add", "a.json"]); - git(repo, ["commit", "-q", "-m", "init"]); - - const host = path.join(os.tmpdir(), `snapshot-cli-${Math.random().toString(36).slice(2)}`); - tmpDirs.push(host); - const scriptDir = path.join(host, dirName); - mkdirSync(scriptDir, { recursive: true }); - const scriptPath = path.join(scriptDir, "git-staged-snapshot.mjs"); - copyFileSync(path.join(SCRIPTS_DIR, "git-staged-snapshot.mjs"), scriptPath); - - const dest = path.join(host, "dest"); - execFileSync(process.execPath, [scriptPath, repo, dest], { stdio: "pipe" }); - - expect(existsSync(path.join(dest, "a.json"))).toBe(true); - }, - CLI_TIMEOUT - ); - } + it( + "目录名同时包含空格和非 ASCII 字符时仍应真正物化暂存快照", + () => { + const repo = makeTempRepo(); + writeFileSync(path.join(repo, "a.json"), JSON.stringify({ v: 1 })); + git(repo, ["add", "a.json"]); + + const host = path.join(os.tmpdir(), `snapshot-cli-${Math.random().toString(36).slice(2)}`); + tmpDirs.push(host); + const scriptDir = path.join(host, "测试 中文目录"); + mkdirSync(scriptDir, { recursive: true }); + const scriptPath = path.join(scriptDir, "git-staged-snapshot.mjs"); + copyFileSync(path.join(SCRIPTS_DIR, "git-staged-snapshot.mjs"), scriptPath); + + const dest = path.join(host, "dest"); + execFileSync(process.execPath, [scriptPath, repo, dest], { stdio: "pipe" }); + + expect(existsSync(path.join(dest, "a.json"))).toBe(true); + }, + CLI_TIMEOUT + ); }); }); diff --git a/src/pages/components/NameAvatar.test.tsx b/src/pages/components/NameAvatar.test.tsx index 5137cff71..1b6b9c967 100644 --- a/src/pages/components/NameAvatar.test.tsx +++ b/src/pages/components/NameAvatar.test.tsx @@ -5,10 +5,6 @@ import { getNameAvatarTone, NameAvatar } from "./NameAvatar"; afterEach(cleanup); describe("NameAvatar 名称头像", () => { - it("相同名称稳定映射到同一组设计令牌", () => { - expect(getNameAvatarTone("ScriptCat")).toEqual(getNameAvatarTone("ScriptCat")); - }); - it("返回 label token 类名而不是硬编码颜色", () => { const tone = getNameAvatarTone("ScriptCat"); expect(tone.bg).toMatch(/^bg-label-(green|blue|purple|orange|rose|teal|amber|indigo)-bg$/); @@ -16,6 +12,13 @@ describe("NameAvatar 名称头像", () => { expect(`${tone.bg} ${tone.text}`).not.toMatch(/#|hsl\(|rgb\(|dark:/); }); + it("哈希为负的种子仍映射到有效令牌", () => { + // "Tampermonkey" 的字符串哈希为负数，覆盖取模索引的负值保护；若退化为单次取模会取到 undefined + const tone = getNameAvatarTone("Tampermonkey"); + expect(tone).toBeDefined(); + expect(tone.bg).toMatch(/^bg-label-\w+-bg$/); + }); + it("渲染时使用尺寸参数和设计令牌类", () => { const { getByText } = render( diff --git a/src/pages/components/ui/empty-state.test.tsx b/src/pages/components/ui/empty-state.test.tsx index d378cb1ef..85feee551 100644 --- a/src/pages/components/ui/empty-state.test.tsx +++ b/src/pages/components/ui/empty-state.test.tsx @@ -1,13 +1,18 @@ -import { render, screen } from "@testing-library/react"; +import { cleanup, render, screen } from "@testing-library/react"; import { Inbox } from "lucide-react"; -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; import { EmptyState } from "./empty-state"; +afterEach(cleanup); + describe("基础空状态组件", () => { - it("渲染图标、标题与说明", () => { - render(); + it("非紧凑模式渲染大图标、加粗标题与说明", () => { + render(); - expect(screen.getByText("暂无数据")).toBeInTheDocument(); + const root = screen.getByTestId("empty"); + expect(root).toHaveClass("gap-3"); + expect(root.querySelector("svg")).toHaveClass("size-10"); + expect(screen.getByText("暂无数据")).toHaveClass("font-medium"); expect(screen.getByText("稍后再试")).toBeInTheDocument(); }); diff --git a/src/pages/options/routes/Agent/components/AgentEmptyState.test.tsx b/src/pages/options/routes/Agent/components/AgentEmptyState.test.tsx deleted file mode 100644 index 87db9d90e..000000000 --- a/src/pages/options/routes/Agent/components/AgentEmptyState.test.tsx +++ /dev/null @@ -1,20 +0,0 @@ -import { describe, it, expect, afterEach } from "vitest"; -import { render, cleanup, screen } from "@testing-library/react"; -import { Server } from "lucide-react"; -import { AgentEmptyState } from "./AgentEmptyState"; - -afterEach(() => cleanup()); - -describe("AgentEmptyState 空状态", () => { - it("复用统一状态屏语义", () => { - render(); - const root = screen.getByTestId("empty-state"); - expect(root).toHaveAttribute("data-slot", "state-screen"); - expect(root).toHaveAttribute("role", "status"); - }); - it("无 action 时仍正常渲染", () => { - render(); - expect(screen.getByText("空")).toBeInTheDocument(); - expect(screen.getByText("说明")).toBeInTheDocument(); - }); -}); diff --git a/src/pages/options/routes/Setting/index.test.tsx b/src/pages/options/routes/Setting/index.test.tsx deleted file mode 100644 index 641f402a4..000000000 --- a/src/pages/options/routes/Setting/index.test.tsx +++ /dev/null @@ -1,56 +0,0 @@ -import { describe, it, expect, vi, beforeAll, afterEach, beforeEach } from "vitest"; -import { render, cleanup } from "@testing-library/react"; -import { MemoryRouter } from "react-router-dom"; -import { initTestLanguage } from "@Tests/initTestLanguage"; -import { mockIntersectionObserver } from "@Tests/mockIntersectionObserver"; -import { mockMatchMedia } from "@Tests/mockMatchMedia"; - -const { get, set } = vi.hoisted(() => ({ - get: vi.fn((key: string) => { - if (key === "cloud_sync") - return Promise.resolve({ enable: false, syncDelete: false, syncStatus: true, filesystem: "webdav", params: {} }); - if (key === "cat_file_storage") return Promise.resolve({ status: "unset", filesystem: "webdav", params: {} }); - if (key === "editor_preferences") - return Promise.resolve({ version: 1, fontSize: 14, mouseWheelScrollSensitivity: 1, smoothScrolling: true }); - if (key === "editor_config") return Promise.resolve("{}"); - if (key === "editor_type_definition") return Promise.resolve("declare const GM_info: unknown;"); - if (key === "enable_eslint") return Promise.resolve(false); - if (key === "eslint_config") return Promise.resolve("{}"); - return Promise.resolve("scriptcat"); - }), - set: vi.fn(), -})); -vi.mock("@App/pages/store/global", async () => { - const { createGlobalStoreMock } = await import("@Tests/mocks/pageStores.ts"); - return createGlobalStoreMock({ systemConfig: { get, set } }); -}); -vi.mock("./sections/GeneralSection", () => ({ GeneralSection: () => null })); -vi.mock("./sections/InterfaceSection", () => ({ InterfaceSection: () => null })); -vi.mock("./sections/SyncSection", () => ({ SyncSection: () => null })); -vi.mock("./sections/UpdateSection", () => ({ UpdateSection: () => null })); -vi.mock("./sections/RuntimeSection", () => ({ RuntimeSection: () => null })); -vi.mock("./sections/SecuritySection", () => ({ SecuritySection: () => null })); -vi.mock("./sections/DeveloperSection", () => ({ DeveloperSection: () => null })); -vi.mock("./sections/DeveloperMonacoEditor", () => ({ - DeveloperMonacoEditor: ({ ariaLabel }: { ariaLabel: string }) =>