From f1617b8c03e9a277bc86db64d68dafee70a0e3f2 Mon Sep 17 00:00:00 2001 From: Musa Musa Date: Wed, 26 Aug 2026 16:50:01 +0100 Subject: [PATCH] fix: three test suites had been silently disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answering "do we have unit and e2e tests" turned up that three of them had not run since the src/ reorganisation. All three fail the same way: a path moved, nothing referenced the new location, and no CI job executed the script, so the breakage was invisible. test:integration → src/sql-integration.test.ts (moved to scenarios/) test:perf → src/limiter.simulation.test.ts (moved to policies/) test:compat → dist/compat/opossum.cjs (moved to adapters/) The compat one is the worst. Fixing the generator was not enough: the shim is written into .opossum-compat/ and that directory is treated as a cache, so every run kept using a shim generated before the move. The suite reported 0 of 0 STALLED for every file, which means the README's headline "362 of 362" claim was unverifiable — and the harness said so out loud rather than passing vacuously, which is the only reason it was recoverable. A generated file must not be cached alongside downloaded ones; the shim is now rewritten every run. 362 of 362 again. Root cause is not the paths, it is that nothing ran them. Coverage, smoke and docs were all in CI and all survived the reorg; these three were not, and a suite that never executes cannot be distinguished from one that passes. All three now run in CI. Plus scripts/check-test-scripts.mjs: every src/… path named in a package.json script must exist. It runs first in `verify` so it fails in a second instead of after a full coverage pass, and it fails if it ever matches zero paths, so it cannot go inert the way the thing it guards did. --- .changeset/stale-test-paths.md | 17 +++++++++++++++++ .github/workflows/ci.yml | 8 ++++++++ package.json | 7 ++++--- scripts/check-test-scripts.mjs | 31 +++++++++++++++++++++++++++++++ scripts/opossum-compat.mjs | 33 ++++++++++++++++++++++++--------- 5 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 .changeset/stale-test-paths.md create mode 100644 scripts/check-test-scripts.mjs diff --git a/.changeset/stale-test-paths.md b/.changeset/stale-test-paths.md new file mode 100644 index 0000000..5d1ff90 --- /dev/null +++ b/.changeset/stale-test-paths.md @@ -0,0 +1,17 @@ +--- +"resilix": patch +--- + +Three test suites had been silently disabled since the `src/` reorganisation, and none of them +ran in CI, so nothing reported it. + +- `test:integration` pointed at `src/sql-integration.test.ts` (now `src/scenarios/`) +- `test:perf` pointed at `src/limiter.simulation.test.ts` (now `src/policies/`) +- `test:compat` generated its opossum shim once and cached it, so every harness kept requiring + `dist/compat/opossum.cjs` after the shim moved to `dist/adapters/`. The whole suite reported + **0 of 0 STALLED** — meaning the README's "362 of 362" claim was unverifiable for days. The + shim is now rewritten on every run, since it is a *generated* file rather than a cached one. + +All three now run in CI, and `pnpm test:paths` fails the build if any path named in a +`package.json` script does not exist. `verify` runs it first, so it fails in a second rather than +after a full coverage pass. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 778b2d8..f34109c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,5 +19,13 @@ jobs: - run: pnpm lint - run: pnpm typecheck - run: pnpm test:coverage + + # These three ran nowhere for days after the src/ reorg moved their targets, so nothing + # noticed that test:integration and test:perf pointed at deleted paths and that the + # opossum harness required dist/compat/opossum.cjs. A suite that is never executed is + # indistinguishable from a suite that passes. + - run: pnpm test:paths + - run: pnpm test:perf + - run: pnpm test:compat - run: pnpm build - run: pnpm check:package diff --git a/package.json b/package.json index ab8f1fa..aa4cb4b 100644 --- a/package.json +++ b/package.json @@ -96,13 +96,14 @@ }, "scripts": { "build": "node -e \"require('fs').rmSync('dist',{recursive:true,force:true})\" && tsup", - "verify": "pnpm lint && pnpm typecheck && pnpm test:coverage && pnpm build && pnpm check:package && pnpm test:smoke && pnpm docs:check", + "verify": "pnpm lint && pnpm test:paths && pnpm typecheck && pnpm test:coverage && pnpm build && pnpm check:package && pnpm test:smoke && pnpm docs:check", "docs:dev": "vitepress dev docs", "docs:build": "vitepress build docs", "docs:check": "vitepress build docs && node scripts/check-links.mjs", "docs:preview": "vitepress preview docs", "dev": "tsup --watch", "test": "vitest run", + "test:paths": "node scripts/check-test-scripts.mjs", "test:watch": "vitest", "test:coverage": "vitest run --coverage", "typecheck": "tsc --noEmit", @@ -112,10 +113,10 @@ "prepublishOnly": "pnpm build && pnpm test", "release": "changeset publish", "ci:version": "changeset version && biome format --write package.json", - "test:integration": "RESILIX_TEST_DATABASE_URL=${RESILIX_TEST_DATABASE_URL:-postgres://postgres:resilix@localhost:5459/resilix_test} vitest run src/sql-integration.test.ts", + "test:integration": "RESILIX_TEST_DATABASE_URL=${RESILIX_TEST_DATABASE_URL:-postgres://postgres:resilix@localhost:5459/resilix_test} vitest run src/scenarios/sql-integration.test.ts", "test:compat": "node scripts/opossum-compat.mjs", "test:smoke": "pnpm build && node scripts/smoke.mjs && node scripts/smoke.cjs", - "test:perf": "RESILIX_PERF=1 vitest run src/limiter.simulation.test.ts" + "test:perf": "RESILIX_PERF=1 vitest run src/policies/limiter.simulation.test.ts" }, "devDependencies": { "@arethetypeswrong/cli": "^0.18.5", diff --git a/scripts/check-test-scripts.mjs b/scripts/check-test-scripts.mjs new file mode 100644 index 0000000..c1bbc78 --- /dev/null +++ b/scripts/check-test-scripts.mjs @@ -0,0 +1,31 @@ +// Every test path named in package.json scripts must exist. +// +// `test:integration` and `test:perf` both pointed at pre-reorg paths for days. Neither runs in +// `verify` or in CI — they need a Postgres and an opt-in env var — so `vitest`'s "No test files +// found" exit code was never observed by anyone. A moved file silently disabled two suites. +// +// Zero dependencies, like everything else here. +import { existsSync, readFileSync } from "node:fs"; + +const { scripts } = JSON.parse(readFileSync("package.json", "utf8")); +const problems = []; +let checked = 0; + +for (const [name, body] of Object.entries(scripts ?? {})) { + // any src/… path with a file extension, wherever it appears in the command + for (const [, path] of String(body).matchAll(/(src\/[\w./-]+\.[cm]?tsx?)/g)) { + checked++; + if (!existsSync(path)) problems.push(`${name} → ${path} (no such file)`); + } +} + +if (checked === 0) { + console.error("✗ no test paths matched — this checker has gone inert"); + process.exit(1); +} +if (problems.length) { + console.error(`✗ ${problems.length} stale path(s) in package.json scripts:\n`); + for (const p of problems) console.error(` ${p}`); + process.exit(1); +} +console.log(`✓ ${checked} test paths in package.json scripts all exist`); diff --git a/scripts/opossum-compat.mjs b/scripts/opossum-compat.mjs index fd15a01..0c26b80 100644 --- a/scripts/opossum-compat.mjs +++ b/scripts/opossum-compat.mjs @@ -54,6 +54,25 @@ const EXCLUDED = { const RAW = (f) => `https://raw.githubusercontent.com/nodeshift/opossum/${REF}/test/${f}`; +/** + * Point opossum's `require('../')` at our build. + * + * Rewritten on EVERY run, not just when the harness is first created. It used to be generated + * once inside fetchSuite() and then cached alongside the downloaded suite — so when the shim + * moved from dist/compat/ to dist/adapters/ during the src/ reorg, every cached harness kept + * requiring a path that no longer existed. The whole suite reported 0 of 0 STALLED, which is a + * generated file being treated as a downloaded one. + */ +const writeShim = () => { + writeFileSync( + join(HARNESS, "shim.cjs"), + `const mod = require(${JSON.stringify(join(ROOT, "dist", "adapters", "opossum.cjs"))}); +module.exports = mod.default ?? mod; +module.exports.default = module.exports; +`, + ); +}; + const fetchSuite = async () => { mkdirSync(join(HARNESS, "test", "browser"), { recursive: true }); for (const f of [...TESTS, "browser/browser-tap.js"]) { @@ -65,14 +84,7 @@ const fetchSuite = async () => { join(HARNESS, "package.json"), `${JSON.stringify({ name: "opossum-compat-harness", private: true, main: "./shim.cjs" }, null, 2)}\n`, ); - // opossum's tests do `require('../')`; point that at our build. - writeFileSync( - join(HARNESS, "shim.cjs"), - `const mod = require(${JSON.stringify(join(ROOT, "dist", "compat", "opossum.cjs"))}); -module.exports = mod.default ?? mod; -module.exports.default = module.exports; -`, - ); + writeShim(); execFileSync("npm", ["install", "--silent", "--no-audit", "--no-fund", "tape"], { cwd: HARNESS, stdio: "ignore", @@ -103,8 +115,11 @@ const main = async () => { if (process.argv.includes("--refresh") || !existsSync(join(HARNESS, "shim.cjs"))) { console.log(`fetching opossum@${REF} test suite…`); await fetchSuite(); + } else { + // The suite is cached; the shim is generated, so refresh it regardless. See writeShim(). + writeShim(); } - if (!existsSync(join(ROOT, "dist", "compat", "opossum.cjs"))) { + if (!existsSync(join(ROOT, "dist", "adapters", "opossum.cjs"))) { console.log("building…"); execFileSync("npm", ["run", "build"], { cwd: ROOT, stdio: "ignore" }); }