From 0bb86e369a46b761207919542f214c46ea82f471 Mon Sep 17 00:00:00 2001 From: Jukka-Matti Turtiainen Date: Sun, 17 May 2026 21:39:33 +0300 Subject: [PATCH 1/2] test(core): retire Math.random() from stackDetection.test.ts + ESLint rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stackDetection.test.ts had 11 Math.random() calls across 7 fixtures, violating the "Tests must be deterministic — never Math.random() in tests" rule from packages/core/CLAUDE.md. Failures were not reproducible across runs. - Replaces every Math.random() with mulberry32(seed) from the existing packages/core/src/__tests__/helpers/stressDataGenerator.ts. Different seed per test (1-6) so test-to-test isolation is preserved. - Adds an ESLint no-restricted-syntax rule scoped to **/__tests__/** that catches Math.random() with an actionable error message. Prevents regression — the previous rule existed only as a CLAUDE.md docstring. All 7 stackDetection tests pass against the seeded data. --- eslint.config.js | 20 +++++++++++ .../core/src/__tests__/stackDetection.test.ts | 34 ++++++++++++------- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index e8d271199..c8e57abce 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -206,6 +206,26 @@ export default [ 'variscout/no-interaction-moderator': 'error', }, }, + // Hard rule: never Math.random() in tests — tests must be deterministic. + // Use mulberry32(seed) from packages/core/src/__tests__/helpers/stressDataGenerator.ts + // (or copy the 9-line helper inline). See .claude/rules/testing.md. + { + files: [ + '**/__tests__/**/*.{ts,tsx}', + '**/*.test.{ts,tsx}', + '**/*.spec.{ts,tsx}', + ], + rules: { + 'no-restricted-syntax': [ + 'error', + { + selector: "CallExpression[callee.object.name='Math'][callee.property.name='random']", + message: + 'Tests must be deterministic — use mulberry32(seed) from packages/core/src/__tests__/helpers/stressDataGenerator.ts instead of Math.random().', + }, + ], + }, + }, // Persistence boundary guard (F1+F2 P7.2, audit R12+R13): // Domain stores and non-persistence app code must not import `dexie` directly. // Persistence access is via @variscout/core HubRepository (pwaHubRepository / diff --git a/packages/core/src/__tests__/stackDetection.test.ts b/packages/core/src/__tests__/stackDetection.test.ts index 33742d18b..0a7fa6dff 100644 --- a/packages/core/src/__tests__/stackDetection.test.ts +++ b/packages/core/src/__tests__/stackDetection.test.ts @@ -1,6 +1,10 @@ import { describe, it, expect } from 'vitest'; import { detectColumns } from '../parser'; import type { DataRow } from '../types'; +import { mulberry32 } from './helpers/stressDataGenerator'; + +// Per @variscout/core test discipline: never Math.random() — use a seeded PRNG +// so failures reproduce deterministically. mulberry32 is the canonical helper. describe('detectColumns stack suggestion', () => { it('should suggest stacking for Finland-style tourism data (83 country columns)', () => { @@ -17,6 +21,7 @@ describe('detectColumns stack suggestion', () => { 'Estonia', 'Norway', ]; + const rng = mulberry32(1); const data: DataRow[] = Array.from({ length: 12 }, (_, i) => { const row: DataRow = { Year: 2020, @@ -26,7 +31,7 @@ describe('detectColumns stack suggestion', () => { Total: 300000 + i * 10000, }; for (const c of countries) { - row[c] = Math.floor(5000 + Math.random() * 50000); + row[c] = Math.floor(5000 + rng() * 50000); } return row; }); @@ -60,11 +65,12 @@ describe('detectColumns stack suggestion', () => { }); it('should not suggest stacking when fewer than 5 numeric columns', () => { + const rng = mulberry32(2); const data: DataRow[] = Array.from({ length: 10 }, () => ({ - A: Math.random() * 100, - B: Math.random() * 100, - C: Math.random() * 100, - D: Math.random() * 100, + A: rng() * 100, + B: rng() * 100, + C: rng() * 100, + D: rng() * 100, Label: 'X', })); @@ -74,10 +80,11 @@ describe('detectColumns stack suggestion', () => { }); it('should assign medium confidence for 5-9 columns', () => { + const rng = mulberry32(3); const data: DataRow[] = Array.from({ length: 10 }, () => { const row: DataRow = { ID: 'X' }; for (let i = 0; i < 7; i++) { - row[`Sensor${i}`] = Math.random() * 100; + row[`Sensor${i}`] = rng() * 100; } return row; }); @@ -89,10 +96,11 @@ describe('detectColumns stack suggestion', () => { }); it('should exclude year-like numeric columns from stack', () => { + const rng = mulberry32(4); const data: DataRow[] = Array.from({ length: 10 }, (_, i) => { const row: DataRow = { Year: 2015 + i }; for (let j = 0; j < 6; j++) { - row[`Country${j}`] = Math.floor(Math.random() * 10000); + row[`Country${j}`] = Math.floor(rng() * 10000); } return row; }); @@ -105,13 +113,14 @@ describe('detectColumns stack suggestion', () => { it('should separate columns with very different magnitudes into clusters', () => { // Mix: 5 columns with values ~100 and 5 columns with values ~1,000,000 + const rng = mulberry32(5); const data: DataRow[] = Array.from({ length: 20 }, () => { const row: DataRow = { ID: 'X' }; for (let i = 0; i < 5; i++) { - row[`Small${i}`] = Math.random() * 100; + row[`Small${i}`] = rng() * 100; } for (let i = 0; i < 5; i++) { - row[`Large${i}`] = Math.random() * 1000000; + row[`Large${i}`] = rng() * 1000000; } return row; }); @@ -123,13 +132,14 @@ describe('detectColumns stack suggestion', () => { }); it('should exclude columns matching strong outcome keywords', () => { + const rng = mulberry32(6); const data: DataRow[] = Array.from({ length: 10 }, () => { const row: DataRow = { - temperature: Math.random() * 100, - pressure: Math.random() * 10, + temperature: rng() * 100, + pressure: rng() * 10, }; for (let i = 0; i < 6; i++) { - row[`Zone${i}`] = Math.random() * 100; + row[`Zone${i}`] = rng() * 100; } return row; }); From 8d0f15baa9be9148390da23717c7367caaca4dc7 Mon Sep 17 00:00:00 2001 From: Jukka-Matti Turtiainen Date: Sun, 17 May 2026 21:40:17 +0300 Subject: [PATCH 2/2] docs(testing): document fake-indexeddb trap + add local TDD cheatsheet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes Tier 1 of the testing-strategy plan. - test/setup.ts: expanded comment explaining the fake-indexeddb requirement, the silent-hang failure mode, and why removing the import is dangerous - packages/stores/CLAUDE.md: concise "## Testing" note explaining the per-package setup.ts (mocks idb-keyval for Zustand persist) and the fake-indexeddb pattern for new Dexie-backed stores - docs/05-technical/implementation/testing.md: new "Local TDD cheatsheet" section covering --ui, --changed, --bail, --reporter=verbose for tighter iteration loops; previously undocumented despite all being load-bearing - root CLAUDE.md: one-line pointer to the cheatsheet (kept terse to respect the size budget; full content lives in the testing.md doc) Tier 1 item 4 (architecture-test scope comment in noCrossInvestigationAggregation) was already in place from prior work — verified, no change needed. (.claude/rules/testing.md also updated locally with the same rule additions, but is gitignored — Claude-only per-developer config.) Tier 2 (Vitest Projects migration) is a separate follow-up PR per the plan. --- .claude/rules/testing.md | 5 ++-- CLAUDE.md | 1 + docs/05-technical/implementation/testing.md | 26 +++++++++++++++++++++ packages/stores/CLAUDE.md | 4 ++++ test/setup.ts | 13 ++++++----- 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 6363ddf16..681519574 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -13,7 +13,8 @@ paths: - **Floats: `toBeCloseTo(value, decimals)`** — never `toBe()`. - **Zustand stores: reset state in `beforeEach`** via `useStore.setState(useStore.getInitialState())`. - **i18n in tests: register your own loader** via `import.meta.glob` before `beforeAll` — never assume global registration. -- **Never `Math.random()`** in stats tests — use seeded PRNG helpers. +- **Never `Math.random()`** in tests — use `mulberry32(seed)` from `packages/core/src/__tests__/helpers/stressDataGenerator.ts` (or copy the 9-line helper if cross-package import is awkward). Seeded PRNGs make failures reproduce deterministically. +- **`fake-indexeddb/auto` is globally installed via root `test/setup.ts`.** Do NOT remove that import — any test that transitively pulls in a Dexie-backed store (e.g. `canvasViewportStore` via Canvas tests in `packages/ui`) will hang silently in jsdom without it. The hang has no stack trace — the test never starts. - **E2E selectors: `data-testid`** only — text/role/class change with i18n + theme. - Known flaky: `packages/hooks/src/__tests__/index.test.ts` under concurrent Turbo load — passes in isolation; retry once before treating as failure. -- **Architecture tests** (structural-absence guards): see `docs/05-technical/implementation/testing.md` "Architecture Tests (Structural-Absence Guards)" — read-once + per-name regex pattern, denylist limits, branded-type follow-up. +- **Architecture tests** (structural-absence guards): see `docs/05-technical/implementation/testing.md` "Architecture Tests (Structural-Absence Guards)" — read-once + per-name regex pattern, denylist limits, branded-type follow-up. Scope is **single-package only** (e.g. the Cpk aggregation guard scans `@variscout/core` only). State the scope honestly in the test's docstring. diff --git a/CLAUDE.md b/CLAUDE.md index 0e130165a..71f02a01d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,6 +18,7 @@ Shared agent map: `docs/llms.txt` - `pnpm test` — all packages (turbo) - `pnpm build` — all packages + apps - `claude --chrome` — enable the **official [Claude for Chrome extension](https://claude.com/claude-for-chrome)** for browser-assisted E2E (drives your real Chrome with your login state). +- Local test cheatsheet (`--ui`, `--changed`, `--bail`, `--reporter=verbose`): see `docs/05-technical/implementation/testing.md` § "Local TDD cheatsheet". ## Where to look diff --git a/docs/05-technical/implementation/testing.md b/docs/05-technical/implementation/testing.md index 2638dd086..6d648a2c6 100644 --- a/docs/05-technical/implementation/testing.md +++ b/docs/05-technical/implementation/testing.md @@ -56,6 +56,32 @@ pnpm --filter @variscout/core test -- --coverage # Chrome Browser Testing claude --chrome # Enable Chrome browser access +``` + +### Local TDD cheatsheet + +Faster iteration than `pnpm test` for tight loops: + +```bash +# One file, one package (substring match on path) +pnpm --filter @variscout/ test -- + +# Browser UI for interactive runs at localhost:51204/__vitest__/ +pnpm --filter @variscout/ test -- --ui + +# Only tests affected by uncommitted changes +pnpm --filter @variscout/ test -- --changed + +# Stop on first failure (pairs well with --changed) +pnpm --filter @variscout/ test -- --bail=1 + +# Per-file runtime — use before reaching for pool tuning +pnpm --filter @variscout/ test -- --reporter=verbose +``` + +Prefer `claude --chrome` over standalone Playwright for iterative UX-level debugging — devtools console + login state are immediately available. + +```bash # Then use prompts like: "Run the staged analysis verification protocol" # Playwright E2E (automated regression) diff --git a/packages/stores/CLAUDE.md b/packages/stores/CLAUDE.md index 6360f5dde..39c8dc2ef 100644 --- a/packages/stores/CLAUDE.md +++ b/packages/stores/CLAUDE.md @@ -37,6 +37,10 @@ pnpm --filter @variscout/stores test ``` +## Testing + +Per-package `src/__tests__/setup.ts` (NOT root `test/setup.ts`) — mocks `idb-keyval` with an in-memory Map for Zustand persist + clears it between tests. New Dexie-backed stores: mirror `canvasViewportStore.test.ts:1` and `import 'fake-indexeddb/auto'` at file top. + ## Related - ADR-041, ADR-064, ADR-065, ADR-078, ADR-080 diff --git a/test/setup.ts b/test/setup.ts index 40f029a58..1918ce3c1 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -1,10 +1,11 @@ import '@testing-library/jest-dom'; -// Install fake-indexeddb globally so tests that pull in Dexie-backed stores -// (e.g. canvasViewportStore via Canvas.test.tsx in packages/ui) don't hang -// waiting for an IndexedDB that jsdom doesn't provide. Previously only the -// stores package + the dedicated canvasViewportStore.test.ts registered this, -// so the @variscout/ui suite stalled on Canvas.test.tsx under concurrent -// turbo load (decision-log line 57, 2026-05-14). +// Install fake-indexeddb globally for ALL test files that load this shared +// setup. Any test that transitively imports a Dexie-backed store — most +// notably `useCanvasViewportStore` (which Canvas tests in packages/ui pull +// in) — will hang silently in jsdom without an IndexedDB implementation. +// The hang has no stack trace: the test never starts. Decision-log line 57 +// (2026-05-14) and `.claude/rules/testing.md` document the trap. +// Do NOT remove this import — leaving the shim global is cheap insurance. import 'fake-indexeddb/auto'; import { vi } from 'vitest';