From 7976e6faf24640fe660c8137ec9ff4fc8625b3d5 Mon Sep 17 00:00:00 2001 From: JongHyeok Park Date: Tue, 30 Jun 2026 10:38:33 +0900 Subject: [PATCH] feat(skills): make tdd-workflow test-runner aware (npm/pnpm/yarn/bun) (#2347) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(skills): make tdd-workflow test-runner aware (npm/pnpm/yarn/bun) Add "Step 0: Detect the Test Runner" so the RED/GREEN cycle no longer hardcodes `npm test`. Distinguishes the package manager from the test runner (a project can install with Bun yet run Jest/Vitest), adds a runner command matrix, and warns about `bun test` (native bun:test runner) vs `bun run test` (runs the package.json script) — a common ESM failure mode. Adds a Bun native test pattern section and links the bun-runtime skill. Applied to both the canonical skills/ copy and the .agents/skills/ Codex subset (manual sync per CONTRIBUTING). * docs(skills): apply / placeholders in tdd-workflow steps Address review feedback on PR #2347: Step 0 instructs the agent to substitute the detected runner command, but Steps 3/5/7, Run Coverage Report, Watch Mode, Pre-Commit, and CI/CD still showed literal `npm test` / `npm run test:coverage` — so an agent reaching those blocks could run npm test on a pnpm/bun project. Replace them with the / / placeholders from Step 0. Left untouched: the plan-handoff allowlist example and the Step 8 evidence-table samples (illustrative, not run-this instructions). Applied to both the canonical and Codex-subset copies. * docs(skills): make pre-commit lint runner-agnostic via placeholder Follow-up to PR #2347 review (CodeRabbit): the pre-commit example still used `npm run lint`, coupling it to npm after test/coverage were made runner-aware. Add a `` column to the Step 0 runner matrix (npm run lint / pnpm lint / yarn lint / bun run lint) and change the Pre-Commit Hook example to ` && `. Applied to both the canonical and Codex-subset copies. * chore: re-trigger CI (flaky windows/node20 npm cell) --- .agents/skills/tdd-workflow/SKILL.md | 71 +++++++++++++++++++++++++--- skills/tdd-workflow/SKILL.md | 71 +++++++++++++++++++++++++--- 2 files changed, 128 insertions(+), 14 deletions(-) diff --git a/.agents/skills/tdd-workflow/SKILL.md b/.agents/skills/tdd-workflow/SKILL.md index 7e61dcef..661a1e58 100644 --- a/.agents/skills/tdd-workflow/SKILL.md +++ b/.agents/skills/tdd-workflow/SKILL.md @@ -48,6 +48,34 @@ ALWAYS write tests first, then implement code to make tests pass. ## TDD Workflow Steps +### Step 0: Detect the Test Runner + +Do not assume `npm test`. The commands in the steps and examples below use ``, ``, and `` as placeholders for the project's actual runner. Resolve them once before starting: + +1. **Run the package-manager detector** (ships with ECC): + + ```bash + node scripts/setup-package-manager.js --detect + ``` + + It resolves the package manager (npm / pnpm / yarn / bun) from, in order: `CLAUDE_PACKAGE_MANAGER`, `.claude/package-manager.json`, the `package.json` `packageManager` field, the lockfile, then global config. + +2. **Distinguish the package manager from the test runner — they are not the same.** A project can use Bun to install dependencies yet still run Jest or Vitest. Inspect `package.json` `scripts.test` and the test files: + - `scripts.test` invokes `jest` / `vitest` -> run through the detected PM (`npm test`, `pnpm test`, `yarn test`, or `bun run test`). + - `scripts.test` is `bun test`, or test files `import { test, expect } from "bun:test"`, or there is no jest/vitest config but Bun is present -> use **Bun's native runner** (`bun test`). See [Bun Native Test Pattern](#bun-native-test-pattern-buntest) below. + +Runner command matrix: + +| Runner | `` | `` | `` | `` | +|--------|----------|----------------|--------------|----------| +| npm | `npm test` | `npm test -- --watch` | `npm run test:coverage` | `npm run lint` | +| pnpm | `pnpm test` | `pnpm test --watch` | `pnpm test:coverage` | `pnpm lint` | +| yarn | `yarn test` | `yarn test --watch` | `yarn test:coverage` | `yarn lint` | +| Bun (script runs jest/vitest) | `bun run test` | `bun run test --watch` | `bun run test:coverage` | `bun run lint` | +| Bun (native `bun:test`) | `bun test` | `bun test --watch` | `bun test --coverage` | `bun run lint` | + +> `bun test` (Bun's built-in runner) is **not** the same as `bun run test` (which runs the `package.json` `test` script). Picking the wrong one is a common failure — e.g. invoking Jest through `npx`/`bun run` in an ESM-only project breaks, while `bun test` runs the suite natively. Confirm which the project expects before the RED gate, then substitute `` / `` everywhere `npm test` appears below. + ### Step 1: Write User Journeys ``` As a [role], I want to [action], so that [benefit] @@ -82,7 +110,7 @@ describe('Semantic Search', () => { ### Step 3: Run Tests (They Should Fail) ```bash -npm test + # Tests should fail - we haven't implemented yet ``` @@ -98,7 +126,7 @@ export async function searchMarkets(query: string) { ### Step 5: Run Tests Again ```bash -npm test + # Tests should now pass ``` @@ -111,7 +139,7 @@ Improve code quality while keeping tests green: ### Step 7: Verify Coverage ```bash -npm run test:coverage + # Verify 80%+ coverage achieved ``` @@ -144,6 +172,35 @@ describe('Button Component', () => { }) ``` +### Bun Native Test Pattern (`bun:test`) + +When the project uses Bun's built-in runner (see [Step 0](#step-0-detect-the-test-runner)), import from `bun:test` and run with `bun test` — not `bun run test`. The API is Jest-like, so `describe` / `it` / `expect` and most matchers carry over. See the `bun-runtime` skill for runtime, install, and bundler details. + +```typescript +import { describe, it, expect, mock } from 'bun:test' +import { searchMarkets } from './search' + +describe('searchMarkets', () => { + it('returns an empty list for an empty query', async () => { + expect(await searchMarkets('')).toEqual([]) + }) + + it('sorts results by similarity score', async () => { + const results = await searchMarkets('election') + expect(results).toEqual([...results].sort((a, b) => b.score - a.score)) + }) +}) +``` + +```bash +bun test # run once (RED/GREEN gate) +bun test --watch # watch mode during development +bun test --coverage # coverage report +``` + +- Mock modules with `mock.module(...)` / `mock(...)` from `bun:test` instead of `jest.mock(...)`. +- Configure coverage thresholds in `bunfig.toml` under `[test]` (e.g. `coverageThreshold`) rather than the Jest `coverageThresholds` config block. + ### API Integration Test Pattern ```typescript import { NextRequest } from 'next/server' @@ -292,7 +349,7 @@ jest.mock('@/lib/openai', () => ({ ### Run Coverage Report ```bash -npm run test:coverage + ``` ### Coverage Thresholds @@ -363,21 +420,21 @@ test('updates user', () => { ### Watch Mode During Development ```bash -npm test -- --watch + # Tests run automatically on file changes ``` ### Pre-Commit Hook ```bash # Runs before every commit -npm test && npm run lint + && ``` ### CI/CD Integration ```yaml # GitHub Actions - name: Run Tests - run: npm test -- --coverage + run: - name: Upload Coverage uses: codecov/codecov-action@v3 ``` diff --git a/skills/tdd-workflow/SKILL.md b/skills/tdd-workflow/SKILL.md index 1e8765c4..03503df1 100644 --- a/skills/tdd-workflow/SKILL.md +++ b/skills/tdd-workflow/SKILL.md @@ -85,6 +85,34 @@ ALWAYS write tests first, then implement code to make tests pass. ## TDD Workflow Steps +### Step 0: Detect the Test Runner + +Do not assume `npm test`. The commands in the steps and examples below use ``, ``, and `` as placeholders for the project's actual runner. Resolve them once before starting: + +1. **Run the package-manager detector** (ships with ECC): + + ```bash + node scripts/setup-package-manager.js --detect + ``` + + It resolves the package manager (npm / pnpm / yarn / bun) from, in order: `CLAUDE_PACKAGE_MANAGER`, `.claude/package-manager.json`, the `package.json` `packageManager` field, the lockfile, then global config. + +2. **Distinguish the package manager from the test runner — they are not the same.** A project can use Bun to install dependencies yet still run Jest or Vitest. Inspect `package.json` `scripts.test` and the test files: + - `scripts.test` invokes `jest` / `vitest` -> run through the detected PM (`npm test`, `pnpm test`, `yarn test`, or `bun run test`). + - `scripts.test` is `bun test`, or test files `import { test, expect } from "bun:test"`, or there is no jest/vitest config but Bun is present -> use **Bun's native runner** (`bun test`). See [Bun Native Test Pattern](#bun-native-test-pattern-buntest) below. + +Runner command matrix: + +| Runner | `` | `` | `` | `` | +|--------|----------|----------------|--------------|----------| +| npm | `npm test` | `npm test -- --watch` | `npm run test:coverage` | `npm run lint` | +| pnpm | `pnpm test` | `pnpm test --watch` | `pnpm test:coverage` | `pnpm lint` | +| yarn | `yarn test` | `yarn test --watch` | `yarn test:coverage` | `yarn lint` | +| Bun (script runs jest/vitest) | `bun run test` | `bun run test --watch` | `bun run test:coverage` | `bun run lint` | +| Bun (native `bun:test`) | `bun test` | `bun test --watch` | `bun test --coverage` | `bun run lint` | + +> `bun test` (Bun's built-in runner) is **not** the same as `bun run test` (which runs the `package.json` `test` script). Picking the wrong one is a common failure — e.g. invoking Jest through `npx`/`bun run` in an ESM-only project breaks, while `bun test` runs the suite natively. Confirm which the project expects before the RED gate, then substitute `` / `` everywhere `npm test` appears below. + ### Step 1: Write User Journeys If a `*.plan.md` file was provided, extract the user journeys and acceptance criteria from that plan first. Only write new journeys for gaps the plan does not cover. @@ -122,7 +150,7 @@ describe('Semantic Search', () => { ### Step 3: Run Tests (They Should Fail) ```bash -npm test + # Tests should fail - we haven't implemented yet ``` @@ -163,7 +191,7 @@ If the repository is under Git, stage the minimal fix now but defer the checkpoi ### Step 5: Run Tests Again ```bash -npm test + # Tests should now pass ``` @@ -191,7 +219,7 @@ Recommended commit message format: ### Step 7: Verify Coverage ```bash -npm run test:coverage + # Verify 80%+ coverage achieved ``` @@ -261,6 +289,35 @@ describe('Button Component', () => { }) ``` +### Bun Native Test Pattern (`bun:test`) + +When the project uses Bun's built-in runner (see [Step 0](#step-0-detect-the-test-runner)), import from `bun:test` and run with `bun test` — not `bun run test`. The API is Jest-like, so `describe` / `it` / `expect` and most matchers carry over. See the `bun-runtime` skill for runtime, install, and bundler details. + +```typescript +import { describe, it, expect, mock } from 'bun:test' +import { searchMarkets } from './search' + +describe('searchMarkets', () => { + it('returns an empty list for an empty query', async () => { + expect(await searchMarkets('')).toEqual([]) + }) + + it('sorts results by similarity score', async () => { + const results = await searchMarkets('election') + expect(results).toEqual([...results].sort((a, b) => b.score - a.score)) + }) +}) +``` + +```bash +bun test # run once (RED/GREEN gate) +bun test --watch # watch mode during development +bun test --coverage # coverage report +``` + +- Mock modules with `mock.module(...)` / `mock(...)` from `bun:test` instead of `jest.mock(...)`. +- Configure coverage thresholds in `bunfig.toml` under `[test]` (e.g. `coverageThreshold`) rather than the Jest `coverageThresholds` config block. + ### API Integration Test Pattern ```typescript import { NextRequest } from 'next/server' @@ -409,7 +466,7 @@ jest.mock('@/lib/openai', () => ({ ### Run Coverage Report ```bash -npm run test:coverage + ``` ### Coverage Thresholds @@ -480,21 +537,21 @@ test('updates user', () => { ### Watch Mode During Development ```bash -npm test -- --watch + # Tests run automatically on file changes ``` ### Pre-Commit Hook ```bash # Runs before every commit -npm test && npm run lint + && ``` ### CI/CD Integration ```yaml # GitHub Actions - name: Run Tests - run: npm test -- --coverage + run: - name: Upload Coverage uses: codecov/codecov-action@v3 ```