diff --git a/.claude/rules/skill-guidance.md b/.claude/rules/skill-guidance.md index 854366fda9..a754a06e5f 100644 --- a/.claude/rules/skill-guidance.md +++ b/.claude/rules/skill-guidance.md @@ -18,6 +18,8 @@ This project has guided skills for common workflows. **Proactively suggest the r | `/scaffold-snowflake-connector` | Add a new Snowflake-connector data source or integration | | `/packages-worker-setup` | First-time setup of packages-db and github-repos-enricher for a new engineer | | `/packages-worker-add-entrypoint` | Scaffold a new sibling worker inside packages_worker (npm, OSV, scorecard, etc.) | +| `/write-unit-tests` | Add or improve Vitest unit tests for business logic, DAL, or server modules | +| `/write-api-e2e-tests` | Add or change Public API e2e / smoke / contract tests | ## Trigger Phrases @@ -57,3 +59,14 @@ This project has guided skills for common workflows. **Proactively suggest the r - "Add a new packages worker", "scaffold a sibling worker", "new entry point in packages_worker" - "Add npm ingestion", "add OSV worker", "add scorecard runner" - Any request to create a new `src/bin/*.ts` worker inside `packages_worker` + +**`/write-unit-tests`** — match any of these intents: +- "Write unit tests", "add a unit test", "cover this with Vitest" +- "Test this function", "add DAL tests", "unit test for affiliations/merges/inference" +- Any request for focused server/unit test coverage (not HTTP/API contract tests) +- Not for `packages_worker` (excluded from `pnpm test:server` for now) + +**`/write-api-e2e-tests`** — match any of these intents: +- "Write API e2e tests", "add smoke tests", "Public API contract tests" +- "Cover this endpoint end-to-end", "API regression test" +- Any request for HTTP/API e2e coverage of Public API behaviour diff --git a/.claude/skills/write-api-e2e-tests/SKILL.md b/.claude/skills/write-api-e2e-tests/SKILL.md new file mode 100644 index 0000000000..9d7e70ebe8 --- /dev/null +++ b/.claude/skills/write-api-e2e-tests/SKILL.md @@ -0,0 +1,66 @@ +--- +name: write-api-e2e-tests +description: > + Write API end-to-end tests. Use when adding or changing API endpoints, or when + the user asks for API e2e, smoke, or contract tests. +allowed-tools: Bash, Read, Glob, Grep, Edit, Write, AskUserQuestion +--- + +# Write API end-to-end tests + +Write or extend API end-to-end tests. + +## When to use + +- New or changed API endpoints. +- User asks for API e2e, smoke, or contract tests. +- Critical API behaviour needs regression coverage. + +## When not to use + +- Domain or SQL correctness → use `write-unit-tests`. +- Temporal, OpenSearch, or other eventual side effects outside the documented API e2e scope. +- Creating a new testing framework or suite style. + +## Source of truth + +Read before writing. Follow these ADRs and existing suite structure; do not +invent a parallel testing style. + +- [ADR-0012](../../../docs/adr/0012-api-e2e-test-architecture.md) — runtime, isolation, supported surfaces, scope, and assertions. +- [ADR-0013](../../../docs/adr/0013-api-e2e-test-suite-design.md) — suite organisation, helpers, and conventions. + +Current default entrypoint: + +- `.github/scripts/public-api-e2e-tests.sh` + +## Workflow + +1. Identify the API surface. Default to Public API unless the user specifies otherwise. +2. Read ADR-0012 and ADR-0013. +3. Add or extend the appropriate suite and register it if required. +4. Run the affected suite locally and fix failures until green. +5. If required fixtures cannot be created through the API, prefer testing supported scenarios and explain any coverage gaps instead of seeding the database directly. +6. Suggest production testability improvements only when they make the API easier to test, and ask before changing production code. + +## Run + +Export the environment variables required by the suite entrypoint. + +```bash +bash .github/scripts/public-api-e2e-tests.sh +``` + +Refer to ADR-0012 and the suite entrypoint for environment setup, reset behaviour, +and local development workflows. + +## Guardrails + +- Keep tests focused on observable API behaviour. + +## Output + +- Suites and cases added +- How to re-run +- Coverage gaps, if any +- Optional testability suggestions diff --git a/.claude/skills/write-unit-tests/SKILL.md b/.claude/skills/write-unit-tests/SKILL.md new file mode 100644 index 0000000000..aecdc54282 --- /dev/null +++ b/.claude/skills/write-unit-tests/SKILL.md @@ -0,0 +1,72 @@ +--- +name: write-unit-tests +description: > + Write focused Vitest unit tests. Use when adding or improving unit tests for + business logic, data access, common services, or other server modules. +allowed-tools: Bash, Read, Glob, Grep, Edit, Write, AskUserQuestion +--- + +# Write unit tests + +Write focused unit tests that follow the project's testing conventions. + +## When to use + +- User asks to add or improve unit tests. +- A change touches high-blast-radius logic (affiliations, merges, identity resolution, timelines, inference). +- A PR needs confidence in a pure or Postgres-backed function. + +## When not to use + +- Public or HTTP contract coverage → use `write-api-e2e-tests`. +- `packages_worker` (excluded from `pnpm test:server` for now; own vitest/packages-db). +- Temporal, Redis, or OpenSearch fixtures (not available yet). +- Broad "increase coverage %" requests without a clear unit under test. + +## Source of truth + +Read before writing. Follow these ADRs; do not invent a parallel testing style. + +- [ADR-0008](../../../docs/adr/0008-how-we-write-unit-tests.md) — scenarios, `describe` grouping, assertions, mocking, and shared setup. +- [ADR-0007](../../../docs/adr/0007-test-factory-primitives-and-defaults.md) — factories and defaults. + +## Workflow + +1. Identify the unit under test (one function or decision path). Colocate tests as `.test.ts`. +2. Read ADR-0007 and ADR-0008. Skim the nearest existing test in the same area if one exists. +3. Compose fixtures using `@crowd/test-kit` (`withQx` for Postgres-backed tests; factories and opt-in defaults per ADR-0007). +4. Write focused scenarios following ADR-0008 (grouping, naming, assertions, and mocking). +5. Run the affected tests and fix failures until green. +6. If production code is difficult to test, suggest a small testability seam and ask before changing production code. + +## Run + +Start the test database when needed: + +```bash +./scripts/cli scaffold up-test +``` + +Run a focused test file: + +```bash +pnpm test:server -- path/to/file.test.ts +``` + +Optional: + +```bash +pnpm test:changed +pnpm test:watch -- path/to/file.test.ts +``` + +## Guardrails + +- Prefer critical behaviours over trivial getters, setters, and thin wrappers. +- Keep production behaviour unchanged unless the user explicitly asks for a testability improvement. + +## Output + +- Scenarios covered +- How to re-run +- Optional testability suggestions diff --git a/docs/adr/0008-how-we-write-unit-tests.md b/docs/adr/0008-how-we-write-unit-tests.md index 3e6f84f2fb..5de6402a73 100644 --- a/docs/adr/0008-how-we-write-unit-tests.md +++ b/docs/adr/0008-how-we-write-unit-tests.md @@ -25,6 +25,8 @@ reuse is real. - Setup should make that story obvious (concrete labels, dates, relationships). - Do not combine unrelated behaviors in a single test. - Prefer fewer tests that fail clearly over many that pass for the wrong reason. +- When a file covers more than one function, group cases with + `describe('', …)` and put named scenarios inside. ### Assertions @@ -33,6 +35,18 @@ reuse is real. - Avoid loose probes (`some`, `greaterThan`, optional finds) unless the claim truly is approximate. +### Mocking + +- Do not mock the data-access layer or the unit under test. Postgres-backed + tests use a real test database and factories; query behavior is part of what + we ship. +- Reach for mocks only at external network boundaries where the real dependency + would hurt reliability or speed (outbound HTTP, LLM clients, Slack/email + senders, Auth0 JWKS in end-to-end tests). +- Control non-determinism in-process when needed (e.g. fake timers for “now”). +- If a test seems to need a mock outside that list, the unit is often too large: + extract the pure part, unit-test it, and let the outer path stay real. + ### What to share | Layer | Where | Belongs |