test(repo): prefer test.each over for/forEach loops in tests (#303)

Convert hand-rolled parameterization loops in test bodies (and around
test definitions) to test.each so each case becomes its own reported
test with pinpointed failure attribution. Document the convention in
.claude/rules/testing.md, including Bun's readonly-array spread gotcha
and the runtime-data exception.

Author: rafa-thayto

.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.

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);
+  });
 });

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");

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", () => {

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", () => {

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(

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<string, string>;
+  devDeps?: Record<string, string>;
+  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);
-  });
-}
+  },
+);