diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index aa288437..0817b834 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -26,4 +26,34 @@ This runs each unit and integration test file as a separate `bun test` subproces Prefer `spyOn()` for mocking, and always restore spies in `afterAll` with `mockRestore()`. +Never use `for` or `forEach` loops inside a single test to verify multiple inputs or cases — use `test.each` (or `it.each` / `describe.each`) so each case is its own reported test case with its own name, setup/teardown, and pinpointed failure output. + +```ts +// ❌ Don't +test("normalizes inputs", () => { + for (const [input, expected] of cases) { + expect(normalize(input)).toBe(expected); + } +}); + +// ✅ Do +test.each(cases)("normalizes %s -> %s", (input, expected) => { + expect(normalize(input)).toBe(expected); +}); +``` + +Bun's `test.each` rejects `readonly`/`as const` arrays via its literal-inferring overload. Spread to a mutable copy so the literal union is preserved in the callback type: + +```ts +const MODES = ["human", "agent"] as const; +test.each([...MODES])("mode %s", (mode) => { + /* mode: "human" | "agent" */ +}); +``` + +Exceptions where a loop in the test body is fine: + +- The iteration itself is the behavior under test (asserting an event fires N times, accumulating state across steps). +- The data being iterated is collected at runtime inside the test and cannot be expressed as a static array at module-load time (e.g. `http.requests` after the action runs, files map captured from a callback). + `mock.module()` is acceptable only when registered at file top, before any consumer of the mocked module is loaded (the integration harness at `packages/cli-core/src/test/integration/lib/harness.ts` and `packages/cli-core/src/lib/credential-store.test.ts` both follow this pattern). In Bun 1.x, `mock.module()` registrations are process-lifetime and will pollute the module registry for any later test file that imports the same module via a non-mocked path, so do not call `mock.module()` from inside `beforeEach`/`describe`/`test`, and do not introduce it in test files that will run alongside files importing the real module. diff --git a/packages/cli-core/src/cli-program.test.ts b/packages/cli-core/src/cli-program.test.ts index a0769a99..97039ca4 100644 --- a/packages/cli-core/src/cli-program.test.ts +++ b/packages/cli-core/src/cli-program.test.ts @@ -379,28 +379,25 @@ describe("help: clerk skill install tip", () => { expect(help).toContain("clerk skill install"); }); - for (const dir of STANDARD_AGENT_DIRS) { - test(`hides the tip when ${dir}/skills/clerk-cli/SKILL.md exists under HOME`, () => { - const target = join(tmpHome, dir, "skills/clerk-cli"); - mkdirSync(target, { recursive: true }); - writeFileSync(join(target, "SKILL.md"), "ok"); - expect(renderHelp()).not.toContain(TIP_SUBSTR); - }); + const AGENT_DIR_CASES = STANDARD_AGENT_DIRS.flatMap((dir) => + (["HOME", "cwd"] as const).map((root) => ({ dir, root })), + ); - test(`hides the tip when ${dir}/skills/clerk-cli/SKILL.md exists under cwd`, () => { - const target = join(tmpCwd, dir, "skills/clerk-cli"); + test.each(AGENT_DIR_CASES)( + "hides the tip when $dir/skills/clerk-cli/SKILL.md exists under $root", + ({ dir, root }) => { + const base = root === "HOME" ? tmpHome : tmpCwd; + const target = join(base, dir, "skills/clerk-cli"); mkdirSync(target, { recursive: true }); writeFileSync(join(target, "SKILL.md"), "ok"); expect(renderHelp()).not.toContain(TIP_SUBSTR); - }); - } + }, + ); - for (const rel of EXTRA_REL_PATHS) { - test(`hides the tip when ${rel} exists under cwd`, () => { - const full = join(tmpCwd, rel); - mkdirSync(dirname(full), { recursive: true }); - writeFileSync(full, "ok"); - expect(renderHelp()).not.toContain(TIP_SUBSTR); - }); - } + test.each([...EXTRA_REL_PATHS])("hides the tip when %s exists under cwd", (rel) => { + const full = join(tmpCwd, rel); + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, "ok"); + expect(renderHelp()).not.toContain(TIP_SUBSTR); + }); }); diff --git a/packages/cli-core/src/commands/init/bootstrap.test.ts b/packages/cli-core/src/commands/init/bootstrap.test.ts index 9690ba15..48554070 100644 --- a/packages/cli-core/src/commands/init/bootstrap.test.ts +++ b/packages/cli-core/src/commands/init/bootstrap.test.ts @@ -31,15 +31,18 @@ describe("BOOTSTRAP_REGISTRY", () => { expect(deps).toContain("vite"); }); - test("each entry produces a non-empty string[] for all package managers", () => { - for (const entry of BOOTSTRAP_REGISTRY) { - for (const pm of packageManagers) { - const cmd = entry.buildCommand(pm, "test-app"); - expect(cmd.length).toBeGreaterThan(0); - expect(cmd.every((arg) => typeof arg === "string")).toBe(true); - } - } - }); + const REGISTRY_PM_PAIRS = BOOTSTRAP_REGISTRY.flatMap((entry) => + packageManagers.map((pm) => ({ dep: entry.dep, entry, pm })), + ); + + test.each(REGISTRY_PM_PAIRS)( + "entry $dep produces a non-empty string[] for package manager $pm", + ({ entry, pm }) => { + const cmd = entry.buildCommand(pm, "test-app"); + expect(cmd.length).toBeGreaterThan(0); + expect(cmd.every((arg) => typeof arg === "string")).toBe(true); + }, + ); test("Next.js uses project name as target directory", () => { const cmd = entryFor("next").buildCommand("bun", "my-cool-app"); diff --git a/packages/cli-core/src/commands/skill/install.test.ts b/packages/cli-core/src/commands/skill/install.test.ts index 05a95da2..a5c04971 100644 --- a/packages/cli-core/src/commands/skill/install.test.ts +++ b/packages/cli-core/src/commands/skill/install.test.ts @@ -121,23 +121,23 @@ const ALL_BUNDLED_FILES = [ ] as const; describe("withStagedClerkSkill version rendering", () => { - test("substitutes CLI_VERSION in every staged file", async () => { - await withStagedClerkSkill("4.5.6", async (stageDir) => { - for (const rel of ALL_BUNDLED_FILES) { + const VERSION_CASES: { version: string | undefined; label: string }[] = [ + { version: "4.5.6", label: "explicit 4.5.6" }, + { version: undefined, label: "undefined → latest" }, + ]; + const VERSION_FILE_CASES = VERSION_CASES.flatMap(({ version, label }) => + ALL_BUNDLED_FILES.map((rel) => ({ version, label, rel })), + ); + + test.each(VERSION_FILE_CASES)( + "substitutes {{CLI_VERSION}} ($label) in $rel", + async ({ version, rel }) => { + await withStagedClerkSkill(version, async (stageDir) => { const content = await readFile(join(stageDir, rel), "utf8"); expect(content, rel).not.toContain("{{CLI_VERSION}}"); - } - }); - }); - - test("resolves undefined version to `latest` in every staged file", async () => { - await withStagedClerkSkill(undefined, async (stageDir) => { - for (const rel of ALL_BUNDLED_FILES) { - const content = await readFile(join(stageDir, rel), "utf8"); - expect(content, rel).not.toContain("{{CLI_VERSION}}"); - } - }); - }); + }); + }, + ); }); describe("bundled SKILL.md frontmatter", () => { diff --git a/packages/cli-core/src/lib/listage.test.ts b/packages/cli-core/src/lib/listage.test.ts index df5869ef..ac184844 100644 --- a/packages/cli-core/src/lib/listage.test.ts +++ b/packages/cli-core/src/lib/listage.test.ts @@ -34,20 +34,20 @@ describe("scrollBounds", () => { expect(scrollBounds(20, 19, 5)).toEqual({ above: 15, below: 0 }); }); - test("above + below + pageSize = totalItems (pageSize=5)", () => { - for (let active = 0; active < 20; active++) { - const { above, below } = scrollBounds(20, active, 5); - expect(above + below + 5).toBe(20); - } - }); - - test("above + below + pageSize = totalItems (pageSize=7, odd)", () => { - // Odd pageSize may drift by ±1 at boundaries but must never be catastrophically wrong - for (let active = 0; active < 20; active++) { - const { above, below } = scrollBounds(20, active, 7); - expect(above + below + 7).toBe(20); - } - }); + // Invariant must hold for any active position and any pageSize — including + // odd pageSizes where above/below may drift by ±1 at boundaries. + const PAGE_SIZES = [5, 7]; + const SCROLL_CASES = PAGE_SIZES.flatMap((pageSize) => + Array.from({ length: 20 }, (_, active) => ({ pageSize, active })), + ); + + test.each(SCROLL_CASES)( + "above + below + pageSize = totalItems (pageSize=$pageSize, active=$active)", + ({ pageSize, active }) => { + const { above, below } = scrollBounds(20, active, pageSize); + expect(above + below + pageSize).toBe(20); + }, + ); }); describe("withScrollIndicators", () => { diff --git a/packages/cli-core/src/lib/users.test.ts b/packages/cli-core/src/lib/users.test.ts index 302a8327..c7f5f33b 100644 --- a/packages/cli-core/src/lib/users.test.ts +++ b/packages/cli-core/src/lib/users.test.ts @@ -56,8 +56,9 @@ describe("users helpers", () => { expect((error as CliError).exitCode).toBe(EXIT_CODE.USAGE); }); - test("parseUsersPayload rejects non-object JSON (arrays, primitives, null)", () => { - for (const input of ['["email@example.com"]', '"just a string"', "42", "null"]) { + test.each(['["email@example.com"]', '"just a string"', "42", "null"])( + "parseUsersPayload rejects non-object JSON: %s", + (input) => { let error: unknown; try { parseUsersPayload(input); @@ -66,8 +67,8 @@ describe("users helpers", () => { } expect(error).toBeInstanceOf(CliError); expect((error as CliError).code).toBe(ERROR_CODE.INVALID_JSON); - } - }); + }, + ); test("redactUsersDisplayPayload masks passwords, codes, and private/unsafe metadata", () => { expect( diff --git a/packages/cli-core/src/test/integration/onboard.test.ts b/packages/cli-core/src/test/integration/onboard.test.ts index 114ea0b7..6b28eff7 100644 --- a/packages/cli-core/src/test/integration/onboard.test.ts +++ b/packages/cli-core/src/test/integration/onboard.test.ts @@ -18,77 +18,91 @@ import { const h = useIntegrationTestHarness(); const MODES = ["human", "agent"] as const; -for (const mode of MODES) { - test.each([ - { - framework: "Next.js", - deps: { next: "15.0.0" }, - expectedKey: "NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - { - framework: "React/Vite", - deps: { react: "19.0.0" }, - devDeps: { vite: "6.0.0" }, - expectedKey: "VITE_CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - { - framework: "Express", - deps: { express: "4.21.0" }, - expectedKey: "CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - { - framework: "Astro", - deps: { astro: "5.0.0" }, - expectedKey: "PUBLIC_CLERK_PUBLISHABLE_KEY", - envFile: ".env", - }, - { - framework: "Expo", - deps: { expo: "52.0.0" }, - expectedKey: "EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - { - framework: "Nuxt", - deps: { nuxt: "3.0.0" }, - expectedKey: "NUXT_PUBLIC_CLERK_PUBLISHABLE_KEY", - expectedSecretKey: "NUXT_CLERK_SECRET_KEY", - envFile: ".env", - }, - { - framework: "TanStack Start", - deps: { "@tanstack/react-start": "1.0.0", react: "19.0.0" }, - expectedKey: "VITE_CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - { - framework: "React Router", - deps: { "react-router": "7.0.0", react: "19.0.0" }, - expectedKey: "VITE_CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - { - framework: "Vue", - deps: { vue: "3.0.0" }, - expectedKey: "VITE_CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - { - framework: "Fastify", - deps: { fastify: "5.0.0" }, - expectedKey: "CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - { - framework: "No framework (fallback)", - deps: { lodash: "4.0.0" }, - expectedKey: "CLERK_PUBLISHABLE_KEY", - envFile: ".env.local", - }, - ])(`$framework (${mode})`, async ({ deps, devDeps, expectedKey, expectedSecretKey, envFile }) => { +interface FrameworkCase { + framework: string; + deps: Record; + devDeps?: Record; + expectedKey: string; + expectedSecretKey?: string; + envFile: string; +} + +const FRAMEWORK_CASES: FrameworkCase[] = [ + { + framework: "Next.js", + deps: { next: "15.0.0" }, + expectedKey: "NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, + { + framework: "React/Vite", + deps: { react: "19.0.0" }, + devDeps: { vite: "6.0.0" }, + expectedKey: "VITE_CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, + { + framework: "Express", + deps: { express: "4.21.0" }, + expectedKey: "CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, + { + framework: "Astro", + deps: { astro: "5.0.0" }, + expectedKey: "PUBLIC_CLERK_PUBLISHABLE_KEY", + envFile: ".env", + }, + { + framework: "Expo", + deps: { expo: "52.0.0" }, + expectedKey: "EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, + { + framework: "Nuxt", + deps: { nuxt: "3.0.0" }, + expectedKey: "NUXT_PUBLIC_CLERK_PUBLISHABLE_KEY", + expectedSecretKey: "NUXT_CLERK_SECRET_KEY", + envFile: ".env", + }, + { + framework: "TanStack Start", + deps: { "@tanstack/react-start": "1.0.0", react: "19.0.0" }, + expectedKey: "VITE_CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, + { + framework: "React Router", + deps: { "react-router": "7.0.0", react: "19.0.0" }, + expectedKey: "VITE_CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, + { + framework: "Vue", + deps: { vue: "3.0.0" }, + expectedKey: "VITE_CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, + { + framework: "Fastify", + deps: { fastify: "5.0.0" }, + expectedKey: "CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, + { + framework: "No framework (fallback)", + deps: { lodash: "4.0.0" }, + expectedKey: "CLERK_PUBLISHABLE_KEY", + envFile: ".env.local", + }, +]; + +const ONBOARD_CASES = MODES.flatMap((mode) => FRAMEWORK_CASES.map((c) => ({ ...c, mode }))); + +test.each(ONBOARD_CASES)( + "$framework ($mode)", + async ({ mode, deps, devDeps, expectedKey, expectedSecretKey, envFile }) => { const pkg = { name: "test-project", dependencies: deps, @@ -118,5 +132,5 @@ for (const mode of MODES) { const plapiCalls = http.requests.filter((r) => r.url.includes("/applications/")); expect(plapiCalls.length).toBeGreaterThan(0); - }); -} + }, +); diff --git a/packages/cli-core/src/test/integration/switch-apps.test.ts b/packages/cli-core/src/test/integration/switch-apps.test.ts index 685f34e3..e0c5714c 100644 --- a/packages/cli-core/src/test/integration/switch-apps.test.ts +++ b/packages/cli-core/src/test/integration/switch-apps.test.ts @@ -19,43 +19,41 @@ import { const h = useIntegrationTestHarness(); const MODES = ["human", "agent"] as const; -for (const mode of MODES) { - test(`re-link from one app to another (${mode})`, async () => { - await Bun.write( - join(h.tempDir, "package.json"), - JSON.stringify({ name: "test", dependencies: { next: "15.0.0" } }), - ); - - const appADev = getInstance(MOCK_APP, "development"); - const appBDev = getInstance(MOCK_APP_B, "development"); - - http.mock({ - [`/applications/${MOCK_APP.application_id}`]: MOCK_APP, - }); - await clerk("--mode", mode, "link", "--app", MOCK_APP.application_id); - - let config = await readConfig(); - expect(config.profiles["github.com/test/project"]!.appId).toBe(MOCK_APP.application_id); - - await clerk("--mode", mode, "env", "pull"); - let env = parseEnvFile(await Bun.file(join(h.tempDir, ".env.local")).text(), ".env.local"); - expect(env.get("NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY")).toBe(appADev.publishable_key); - - await clerk("--mode", mode, "unlink", "--yes"); - config = await readConfig(); - expect(config.profiles["github.com/test/project"]).toBeUndefined(); - - http.mock({ - [`/applications/${MOCK_APP_B.application_id}`]: MOCK_APP_B, - }); - await clerk("--mode", mode, "link", "--app", MOCK_APP_B.application_id); - - config = await readConfig(); - expect(config.profiles["github.com/test/project"]!.appId).toBe(MOCK_APP_B.application_id); - - await clerk("--mode", mode, "env", "pull"); - env = parseEnvFile(await Bun.file(join(h.tempDir, ".env.local")).text(), ".env.local"); - expect(env.get("NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY")).toBe(appBDev.publishable_key); - expect(env.get("CLERK_SECRET_KEY")).toBe(appBDev.secret_key); +test.each([...MODES])("re-link from one app to another (%s)", async (mode) => { + await Bun.write( + join(h.tempDir, "package.json"), + JSON.stringify({ name: "test", dependencies: { next: "15.0.0" } }), + ); + + const appADev = getInstance(MOCK_APP, "development"); + const appBDev = getInstance(MOCK_APP_B, "development"); + + http.mock({ + [`/applications/${MOCK_APP.application_id}`]: MOCK_APP, }); -} + await clerk("--mode", mode, "link", "--app", MOCK_APP.application_id); + + let config = await readConfig(); + expect(config.profiles["github.com/test/project"]!.appId).toBe(MOCK_APP.application_id); + + await clerk("--mode", mode, "env", "pull"); + let env = parseEnvFile(await Bun.file(join(h.tempDir, ".env.local")).text(), ".env.local"); + expect(env.get("NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY")).toBe(appADev.publishable_key); + + await clerk("--mode", mode, "unlink", "--yes"); + config = await readConfig(); + expect(config.profiles["github.com/test/project"]).toBeUndefined(); + + http.mock({ + [`/applications/${MOCK_APP_B.application_id}`]: MOCK_APP_B, + }); + await clerk("--mode", mode, "link", "--app", MOCK_APP_B.application_id); + + config = await readConfig(); + expect(config.profiles["github.com/test/project"]!.appId).toBe(MOCK_APP_B.application_id); + + await clerk("--mode", mode, "env", "pull"); + env = parseEnvFile(await Bun.file(join(h.tempDir, ".env.local")).text(), ".env.local"); + expect(env.get("NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY")).toBe(appBDev.publishable_key); + expect(env.get("CLERK_SECRET_KEY")).toBe(appBDev.secret_key); +});