Skip to content

Commit c0b70cb

Browse files
authored
Merge pull request #3852 from AtCoder-NoviSteps/#3847
perf(test): switch vitest default env to node, move jsdom to 3 DOM files
2 parents 2dc36e6 + 1d5f3fe commit c0b70cb

9 files changed

Lines changed: 197 additions & 542 deletions

File tree

.claude/rules/testing.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,52 @@ E2E files: **must** use `.spec.ts` extension (`.test.ts` not detected).
2828

2929
Route unit tests: `src/routes/**/*.test.ts` is included by `vite.config.ts`. **Never use `+` as a filename prefix** — SvelteKit reserves it and `pnpm check` will error. Name route test files `page_server.test.ts`, not `+page.server.test.ts`.
3030

31+
## Test Environment
32+
33+
Default is `node` (set in `vite.config.ts`). Only files touching the real DOM (`window` / `document` / `localStorage`) opt in with a top-of-file `// @vitest-environment jsdom`. **Never set jsdom globally** — most tests are pure units and per-file jsdom construction is ~5.5x slower.
34+
35+
### Toggling `browser` per describe
36+
37+
**Never register `vi.mock('$app/environment')` twice in one file.** Every `vi.mock` is hoisted above the imports and runs once before any test, so a `{ browser: false }` mock written inside an SSR `describe`/`beforeEach` silently overwrites the `{ browser: true }` one at the top — the whole file ends up pinned to `browser = false`, the localStorage branches never execute, and tests that "verify" them become false-positives while still passing.
38+
39+
Use **one dynamic mock** driven by a `vi.hoisted` flag, and toggle the flag in `beforeEach`. Default the flag to `false` so singletons constructed at import time stay SSR-safe:
40+
41+
```typescript
42+
// @vitest-environment jsdom
43+
const browserState = vi.hoisted(() => ({ value: false }));
44+
45+
vi.mock('$app/environment', () => ({
46+
get browser() {
47+
return browserState.value;
48+
},
49+
}));
50+
51+
describe('MyStore', () => {
52+
let store: MyStore;
53+
54+
beforeEach(() => {
55+
browserState.value = true;
56+
localStorage.clear();
57+
store = new MyStore(); // constructed after the flag flips — reads localStorage
58+
});
59+
//
60+
});
61+
62+
describe('MyStore in SSR', () => {
63+
beforeEach(() => {
64+
browserState.value = false;
65+
});
66+
//
67+
});
68+
```
69+
70+
**Never assert a browser branch on the import-time singleton.** The flag is `false` while the module graph is evaluated, so the exported singleton is always built in SSR mode and never touches localStorage — asserting on it under `browser = true` is the same false-positive as the double-`vi.mock` above. Construct a fresh instance inside the browser `describe` (which is why the store class, not just the singleton, needs a named export). The singleton is still worth one assertion: that import-time construction is SSR-safe.
71+
72+
In jsdom files, do **not** stub localStorage with `vi.stubGlobal` — use jsdom's real `Storage` and assert on state (`localStorage.getItem(key)`), not on spy calls. Cover both SSR guards with separate cases, since one assertion cannot carry both:
73+
74+
- **read guard**: pre-seed localStorage, construct a fresh store, expect the _default_ (the pre-seeded value stays in storage, so do not expect `getItem(key)` to be null here)
75+
- **write guard**: start from empty localStorage, call the setter, expect `getItem(key)` to still be null
76+
3177
## Unit Testing Patterns
3278

3379
### Assertions

compose.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ services:
66
- '5555:5555'
77
tty: true
88
volumes:
9-
- .:/usr/src/app:cached
10-
- ./node_modules:/usr/src/app/node_modules:cached
9+
- .:/usr/src/app
10+
- ./node_modules:/usr/src/app/node_modules
1111
environment:
1212
- NODE_ENV=development
1313
- DATABASE_URL=postgresql://db_user:db_password@db:5432/test_db?pgbouncer=true&connection_limit=10&connect_timeout=60&statement_timeout=60000 # Note: Local server cannot start if port is set to db:6543.

package.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
"@sveltejs/vite-plugin-svelte": "7.2.0",
3434
"@tailwindcss/forms": "0.5.11",
3535
"@tailwindcss/vite": "4.3.2",
36-
"@testing-library/jest-dom": "6.9.1",
3736
"@types/gtag.js": "0.0.20",
3837
"@types/jsdom": "28.0.3",
3938
"@typescript-eslint/eslint-plugin": "8.63.0",
@@ -77,8 +76,6 @@
7776
"@lucia-auth/adapter-prisma": "3.0.2",
7877
"@lucide/svelte": "1.24.0",
7978
"@prisma/client": "5.22.0",
80-
"@testing-library/svelte": "5.4.2",
81-
"@types/jest": "30.0.0",
8279
"@types/node": "25.9.5",
8380
"debug": "4.4.3",
8481
"lucia": "2.7.7",

0 commit comments

Comments
 (0)