diff --git a/server/package.json b/server/package.json index 9bc9042..417a9cf 100644 --- a/server/package.json +++ b/server/package.json @@ -10,7 +10,7 @@ "swagger": "node swagger/swagger.js", "routes:manifest": "node scripts/routeManifest.js", "atlas:import": "node scripts/importSpawnAtlas.js", - "test": "node --test" + "test": "node --test --require ./test/_setup.js" }, "keywords": [ "express", diff --git a/server/src/utils/db.js b/server/src/utils/db.js index 713784b..86f63c9 100644 --- a/server/src/utils/db.js +++ b/server/src/utils/db.js @@ -87,7 +87,15 @@ async function ensureCoreSchema({ retries, delayMs }) { } } +// Idempotent: `pool.end()` throws "pool is already closed" on a second call, and +// closing twice is normal rather than exceptional — a SIGINT followed by a +// SIGTERM reaches the shutdown handler twice, and the test harness closes the +// pool for every file on top of the suites that close it themselves. A teardown +// that fails because it had already succeeded is noise. +let closed = false async function close() { + if (closed) return + closed = true await pool.end() } diff --git a/server/test/_helper.js b/server/test/_helper.js index 3baf6c8..0bad8b0 100644 --- a/server/test/_helper.js +++ b/server/test/_helper.js @@ -14,7 +14,14 @@ async function startApp(configure) { const { port } = server.address() return { url: `http://127.0.0.1:${port}`, - close: () => new Promise((resolve) => server.close(resolve)), + // `server.close()` stops accepting and waits for open connections to end on + // their own — and node's global fetch keeps its sockets alive, so nothing + // ever ends them. The listener then outlives the test that made it, which + // used to be invisible because the pool held the process open anyway. + close: () => new Promise((resolve) => { + server.closeAllConnections() + server.close(resolve) + }), } } diff --git a/server/test/_setup.js b/server/test/_setup.js new file mode 100644 index 0000000..81fa20c --- /dev/null +++ b/server/test/_setup.js @@ -0,0 +1,42 @@ +// ── Test-process setup, loaded into every test file ──────────────────────── +// +// `npm test` passes this with `--require`, so it runs before the test file it +// is hosting — which is the only moment early enough to matter, because +// `utils/db.js` builds its mariadb pool at REQUIRE time. +// +// It fixes two things that were per-file conventions, and therefore held only +// as well as the next test file remembered them: +// +// 1. **Nothing in the suite may reach a real database.** Without this, a file +// that forgot the two `process.env` lines picked up `server/.env` through +// db.js's own `dotenv.config()` and pooled five live connections to the +// developer's MariaDB. The tests still passed — they stub their models — +// so the only symptom was the process never exiting, plus five connections +// held for as long as the worker lived. Thirty stranded workers is 150 +// connections, which is the whole server's limit. +// 2. **The pool is closed when the file's tests are done**, so the process can +// exit at once instead of waiting out the driver's connect retries. +// +// `dotenv` does not overwrite variables that already exist, so pinning the dead +// port here beats `.env` while still letting an explicit `DB_PORT=… npm test` +// through for anyone who deliberately wants a live database. +const { after } = require('node:test') + +process.env.DB_HOST = process.env.DB_HOST || '127.0.0.1' +process.env.DB_PORT = process.env.DB_PORT || '59999' + +// A root-level hook, registered before the test file is even read, so it runs +// once after everything in that file. +// +// Only in the per-file child processes. `--require` is inherited by the runner +// process too, and registering a root hook there gives node:test a second root +// context to report — an empty "tests 0 / pass 0" summary printed after the real +// one, which reads like a suite that silently ran nothing. +if (process.env.NODE_TEST_CONTEXT) { + after(async () => { + // Resolved rather than required: a file that never touched the database must + // not have a pool built for it here just so this can close one. + const cached = require.cache[require.resolve('../src/utils/db')] + if (cached) await cached.exports.close() + }) +}