Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .claude/skills/contributing-to-loopover/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ jobs run only if their path filter matched; on push to `main`, everything runs.
| ui → tests | vitest jsdom (UI) | `npm run ui:test` | failing UI component test |
| ui → build | UI build | `npm run ui:build` | build failure (note: it re-runs `ui:openapi` internally) |

**The Node-version guard: `test/helpers/vitest-global-setup-node-version.ts`, not just `pretest*`.**
Every `vitest.config.ts` in the repo (root, `vitest.workers.config.ts`, and every workspace with its own
config) wires this as `globalSetup`, so it runs once before any test file regardless of how vitest was
invoked — including a direct `npx vitest run test/unit/<file>.test.ts`, which this doc's own §6 ("Iterate,
then verify") recommends for fast iteration. `scripts/check-node-version.mjs`'s `pretest*` hooks
(`package.json`) only cover 5 high-traffic npm script names (`test`, `test:ci`, `test:coverage`,
`test:workers`, `ui:test`) as a genuinely-faster fail there (before npm even spawns vitest) — they are a
nicety on top of the globalSetup guarantee, not a substitute for it. An earlier version of this guard was
`pretest*`-only and missed 8 of 12 vitest-invoking script names, plus every direct `npx vitest` call
structurally (caught by a repo-wide audit shortly after it shipped, #7619-follow-up) — if you add a new
top-level test script or workspace, you do not need to remember a matching `pretest*` entry; the
`globalSetup` wiring already covers it.

**Verifying the built artifact actually serves — do not use `apps/loopover-ui`'s `preview` in the raw
TanStack Start template sense.** `apps/loopover-ui` targets nitro's `cloudflare-module` preset
(`vite.config.ts`), which repackages the server build as `dist/server/index.mjs`. `@tanstack/start-plugin-core`'s
Expand Down
1 change: 1 addition & 0 deletions apps/loopover-miner-extension/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export default defineConfig({
test: {
environment: "node",
globals: true,
globalSetup: ["../../test/helpers/vitest-global-setup-node-version.ts"],
include: ["test/**/*.test.ts"],
coverage: {
provider: "v8",
Expand Down
1 change: 1 addition & 0 deletions apps/loopover-miner-ui/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export default defineConfig({
test: {
environment: "jsdom",
globals: true,
globalSetup: ["../../test/helpers/vitest-global-setup-node-version.ts"],
setupFiles: ["./vitest.setup.ts"],
include: ["src/**/*.test.{ts,tsx}"],
coverage: {
Expand Down
1 change: 1 addition & 0 deletions apps/loopover-ui/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export default defineConfig({
test: {
environment: "jsdom",
globals: true,
globalSetup: ["../../test/helpers/vitest-global-setup-node-version.ts"],
setupFiles: ["./vitest.setup.ts"],
include: ["src/**/*.test.{ts,tsx}"],
},
Expand Down
1 change: 1 addition & 0 deletions packages/loopover-ui-kit/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export default defineConfig({
test: {
environment: "jsdom",
globals: true,
globalSetup: ["../../test/helpers/vitest-global-setup-node-version.ts"],
setupFiles: ["./vitest.setup.ts"],
include: ["src/**/*.test.{ts,tsx}"],
},
Expand Down
14 changes: 12 additions & 2 deletions scripts/check-node-version.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,18 @@
// followed by simply switching the active `node` (nvm/homebrew default change) with no reinstall, sails
// straight past engine-strict on every later `npm run`. That's exactly the shape of gap that let the
// Node 26 jsdom/localStorage bug (#7592/#7597/#7612) go unnoticed the first two times: a pile of
// confusing downstream test failures instead of one clear "wrong Node version" message up front. Wired as
// a `pretest*` hook (see package.json) on the commands people actually run vitest through.
// confusing downstream test failures instead of one clear "wrong Node version" message up front.
//
// The actual, complete guarantee is test/helpers/vitest-global-setup-node-version.ts, wired as every
// vitest.config.ts's `globalSetup` (root, workers, and every workspace with its own config) -- it covers
// every invocation path, including a direct `npx vitest run test/unit/<file>.test.ts` (which the
// contributing skill docs explicitly recommend for fast iteration), not just specific npm script names.
// This module is ALSO wired as a `pretest*` hook (see package.json) on the highest-traffic commands
// (test, test:ci, test:coverage, test:workers, ui:test) as a genuinely-faster fail there -- it runs
// before npm even spawns vitest, vs. globalSetup which still pays vitest's own startup cost first -- but
// that hook is a nicety on top of the globalSetup guarantee, not a substitute for it; it was originally
// (incompletely) the only mechanism, missing 8 of 12 vitest-invoking script names (#7592-class gap,
// caught by a repo-wide audit after #7619 shipped).
import { readFileSync } from "node:fs";
import { fileURLToPath } from "node:url";
import semver from "semver";
Expand Down
20 changes: 20 additions & 0 deletions test/helpers/vitest-global-setup-node-version.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { checkNodeVersion } from "../../scripts/check-node-version.mjs";

/**
* Vitest `globalSetup` -- the only mechanism that covers *every* vitest invocation path, not just
* specific npm script names. scripts/check-node-version.mjs's `pretest*` hooks (package.json) only fire
* for the exact npm script names they're wired to -- they can never cover a direct
* `npx vitest run test/unit/<file>.test.ts` invocation, which the contributing skill docs explicitly
* recommend for fast iteration (reference.md, SKILL.md). A globalSetup runs once before any test file,
* regardless of how vitest was invoked, so this is the real backstop; the pretest hooks are a
* (redundant but harmless) earlier fail for the specific commands they cover.
*/
export default function setup(): void {
const { ok, requiredRange, nodeVersion } = checkNodeVersion();
if (!ok) {
throw new Error(
`Running Node ${nodeVersion}, but this repo requires ${requiredRange} (see .nvmrc / package.json engines). ` +
"Switch to the pinned Node version (e.g. `nvm use`) before running tests.",
);
}
}
1 change: 1 addition & 0 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default defineConfig({
environment: "node",
globals: true,
testTimeout: 15000,
globalSetup: ["./test/helpers/vitest-global-setup-node-version.ts"],
// Retry a failed test once before failing the run. The loopover gate auto-CLOSES a contributor PR
// on a red required CI, so a single transient flake must not kill an honest PR; a deterministic
// failure still fails both attempts (and vitest flags the retried test as flaky so it stays visible).
Expand Down
1 change: 1 addition & 0 deletions vitest.workers.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export default defineConfig({
],
test: {
globals: true,
globalSetup: ["./test/helpers/vitest-global-setup-node-version.ts"],
// Retry once before failing — a transient flake must not red the required CI and one-shot-close a PR.
retry: 1,
include: ["test/workers/**/*.test.ts"],
Expand Down
Loading