diff --git a/backend/TESTING.md b/backend/TESTING.md index 4a1b677579..52377bb1bf 100644 --- a/backend/TESTING.md +++ b/backend/TESTING.md @@ -83,6 +83,7 @@ The branch is controlled by `process.env.INTEGRATION_TEST === 'true'`. `__tests_ - **Tier 0 tests don't cross-import `mongoServer` / `pgDb`.** The real-services branch doesn't export them. Use the helpers; if you need direct access, add a narrow helper in `testUtils.js` that works in both tiers. - **Real PG needs `pgcrypto` for `gen_random_uuid()`.** `setupPgDb` creates the extension for Tier 1 — don't call `gen_random_uuid()` in a test that only runs under Tier 0 unless you're also registering the pg-mem function. - **FK ordering matters under real PG.** `pod_members.pod_id` and `messages.pod_id` reference `pods(id) ON DELETE CASCADE`. Tests that insert raw rows must insert into `pods` first. `clearPgDb()` uses `TRUNCATE … CASCADE` to sidestep this on teardown. +- **pg-mem's FK actions are pinned, and two are not PostgreSQL.** `__tests__/service/pgMemFkActions.test.js` measures them. `ON DELETE CASCADE`, `ON DELETE SET NULL`, `NO ACTION` and `RESTRICT` match PostgreSQL for the shipped schema. `ON UPDATE CASCADE` is rejected rather than cascaded, and `ON DELETE SET DEFAULT` sets NULL rather than the default. A Tier 0 test that depends on either of those is testing pg-mem, not production. Assert that path in Tier 1 (real PG), or change the fixture. The pinned expectations follow the PostgreSQL docs and have not yet been confirmed against a live server. - **Timeouts.** Real Mongo operations are slower than in-memory. `jest.setTimeout(30000)` is set globally in `__tests__/setup.js`; avoid hardcoded shorter timeouts in Tier 1 tests. **A timeout in a cold parallel run is contention before it is a regression**: nine `MongoMemoryServer` boots in one cold pass can push a suite past the global 30 s on their own, and the same nine suites then pass three runs in a row (95/95 at default workers, 95/95 at `-w 2`, and the suite that timed out passing 15/15 alone — that third figure is one suite, not nine). Re-run the suite alone before reading the timeout as the diff's. - **New route registration? It needs a rate limiter ahead of auth.** `__tests__/unit/routes/routeRateLimitGuard.test.js` scans every `router.(path, …)` under `routes/` and fails when a registration has no `*RateLimit*` middleware, or has one behind `auth`/`agentRuntimeAuth`/`dualAuth` (the auth lookup is the DB work CodeQL's js/missing-rate-limiting flags). A file-level `router.use(auth)` counts as auth for every route after it, the last argument is the handler (never a limiter), and `router.route(path)` chains are scanned per verb. Shape to copy: `routes/agentHooks.ts`. Pre-existing violations sit in `routeRateLimitGuard.baseline.json`, which may only shrink — fix the route, never extend the list; delete a row once its route is compliant. - **The cast performed by mongoose's array wrapper is part of the measurement, and two mechanisms return the same `true`.** `pod.members` is wrapped in a `MongooseArray` on a hydrated document, so `doc.members.includes('')` is **true** (the wrapper casts) while `[objectId].includes('')` over the same values is **false**. But `JSON.parse(JSON.stringify(doc)).members.includes('')` is **also true** — for a different reason: serialisation turned the values into hex *strings*. So a `true` can come from the cast or from the values having stopped being ObjectIds, and the two are indistinguishable in a green run: say in the test which mechanism produced it, because a `true` from a serialised fixture is a fact about the fixture and proves nothing about the real document. (`doc.toObject()` is the third shape and answers **false**.) Name the mechanism precisely, because the name sends the next reader to a specific place in mongoose: `pod.members` is a genuine `Array` that mongoose has patched (`Array.isArray` true, `isMongooseArray` true) — **not** a `DocumentArray`, whose marker `isMongooseDocumentArray` is *absent* here (`undefined`, not `false`), and which is the subdocument-array case this schema does not use. So the cast comes from mongoose's array patching; a plain array literal carries neither marker and returns `false`. And print the value raw, because `!!` in a probe coerces `undefined` to `false` — three distinguishable states into two, with the printed result still looking like a measurement; @vera's first measurement of this property was reported as `false` for exactly that reason, and the same error one layer down is what this bullet is about. **`constructor.name` is not a shape control either**: it reads `'Array'` for both a patched array and a plain one, so it fails silently where `isMongooseArray` discriminates. (Measured on mongoose 7.8.6 with both predicates, not inferred from the property name.) *(Earned 2026-09-27, TASK-166/#1945, from two different mistakes. @kai's fixture declared an unlisted creator and `Pod`'s pre-save hook silently repaired it — the error was in the fixture, not in the assertion, and the state that reached the assertion was never the state the test named. @vera ran the hydrated document and the bare array in one probe as a deliberate contrast, which is what made the two mechanisms separable, and contributed the JSON third shape. The rule is the same for both: the instrument, not the answer, is what the green run certifies.)* diff --git a/backend/__tests__/service/pgMemFkActions.test.js b/backend/__tests__/service/pgMemFkActions.test.js new file mode 100644 index 0000000000..f3a45186a2 --- /dev/null +++ b/backend/__tests__/service/pgMemFkActions.test.js @@ -0,0 +1,147 @@ +/** + * Pins how pg-mem enforces foreign-key actions, so a Tier 0 test that leans on + * ON DELETE / ON UPDATE behaviour is leaning on something we have measured. + * + * Two halves: + * 1. The shipped DDL (schema.sql via schemaTable) — the FKs production uses. + * These must keep matching PostgreSQL, or Tier 0 tests lie about cascades. + * 2. Isolated tables for the actions schema.sql does not use. Where pg-mem + * diverges from PostgreSQL, the test pins the divergence and says so, so a + * pg-mem upgrade that changes it fails here instead of silently. + * + * Expected values follow the PostgreSQL docs (ddl-constraints, "Referential + * Actions"). No real PostgreSQL was available when this was written, so those + * expectations are not yet confirmed against a live server. Tier 1 (real PG) + * is the place to confirm them. + */ +const crypto = require('crypto'); +const { newDb } = require('pg-mem'); +const { applyTable } = require('../utils/schemaTable'); + +const freshPool = () => { + const db = newDb(); + db.public.registerFunction({ + name: 'gen_random_uuid', + implementation: () => crypto.randomUUID(), + }); + const { Pool } = db.adapters.createPg(); + return new Pool(); +}; + +// Runs `action` against a parent/child pair built with `childFk`, returns what +// the child's pid became, or 'reject' when pg-mem refuses the statement. +const probe = async ({ childFk, action }) => { + const pool = freshPool(); + await pool.query('CREATE TABLE p (id int PRIMARY KEY)'); + await pool.query(`CREATE TABLE c (id int PRIMARY KEY, pid int ${childFk})`); + await pool.query('INSERT INTO p VALUES (1), (2)'); + await pool.query('INSERT INTO c VALUES (10, 1)'); + try { + await pool.query(action); + const r = await pool.query('SELECT pid FROM c WHERE id = 10'); + return r.rows[0].pid === null ? 'null' : r.rows[0].pid; + } catch (e) { + return 'reject'; + } +}; + +describe('pg-mem FK actions on the shipped schema.sql DDL', () => { + let pool; + + beforeEach(async () => { + pool = freshPool(); + await applyTable(pool, 'pods'); + await applyTable(pool, 'pod_members'); + await applyTable(pool, 'messages'); + await pool.query("INSERT INTO pods (id, name, type, created_by) VALUES ('p1', 'n', 't', 'u')"); + }); + + test('ON DELETE CASCADE removes pod_members when the pod is deleted', async () => { + await pool.query("INSERT INTO pod_members (pod_id, user_id) VALUES ('p1', 'u1')"); + await pool.query("DELETE FROM pods WHERE id = 'p1'"); + const r = await pool.query('SELECT count(*)::int AS n FROM pod_members'); + expect(r.rows[0].n).toBe(0); + }); + + test('ON DELETE SET NULL nulls reply_to_message_id and keeps the reply row', async () => { + await pool.query("INSERT INTO messages (id, pod_id, user_id, content) VALUES (1, 'p1', 'u', 'a')"); + await pool.query( + `INSERT INTO messages (id, pod_id, user_id, content, reply_to_message_id) + VALUES (2, 'p1', 'u', 'b', 1)`, + ); + await pool.query('DELETE FROM messages WHERE id = 1'); + const r = await pool.query('SELECT reply_to_message_id AS r FROM messages WHERE id = 2'); + expect(r.rows[0].r).toBeNull(); + }); + + test('ON DELETE SET NULL on thread_root_id keeps the child row', async () => { + await pool.query("INSERT INTO messages (id, pod_id, user_id, content) VALUES (1, 'p1', 'u', 'root')"); + await pool.query( + `INSERT INTO messages (id, pod_id, user_id, content, thread_root_id) + VALUES (2, 'p1', 'u', 'reply', 1)`, + ); + await pool.query('DELETE FROM messages WHERE id = 1'); + const r = await pool.query('SELECT thread_root_id AS t FROM messages WHERE id = 2'); + expect(r.rows[0].t).toBeNull(); + }); + + test('a child row pointing at a missing parent is rejected on insert', async () => { + await expect( + pool.query("INSERT INTO pod_members (pod_id, user_id) VALUES ('nope', 'u')"), + ).rejects.toThrow(/foreign key/i); + }); +}); + +describe('pg-mem FK actions outside the shipped DDL', () => { + test('NO ACTION (the default) rejects a parent delete', async () => { + const got = await probe({ + childFk: 'REFERENCES p(id)', + action: 'DELETE FROM p WHERE id = 1', + }); + expect(got).toBe('reject'); + }); + + test('RESTRICT rejects a parent delete', async () => { + const got = await probe({ + childFk: 'REFERENCES p(id) ON DELETE RESTRICT', + action: 'DELETE FROM p WHERE id = 1', + }); + expect(got).toBe('reject'); + }); + + test('ON UPDATE SET NULL nulls the child on a parent key change', async () => { + const got = await probe({ + childFk: 'REFERENCES p(id) ON UPDATE SET NULL', + action: 'UPDATE p SET id = 3 WHERE id = 1', + }); + expect(got).toBe('null'); + }); + + test('ON UPDATE NO ACTION (the default) rejects a parent key change', async () => { + const got = await probe({ + childFk: 'REFERENCES p(id)', + action: 'UPDATE p SET id = 3 WHERE id = 1', + }); + expect(got).toBe('reject'); + }); + + // DIVERGENCE — PostgreSQL cascades the key change to the child (child = 3). + // pg-mem rejects the update. Pinned so Tier 0 code cannot rely on ON UPDATE CASCADE. + test('DIVERGENCE: pg-mem rejects ON UPDATE CASCADE (PostgreSQL cascades to 3)', async () => { + const got = await probe({ + childFk: 'REFERENCES p(id) ON UPDATE CASCADE', + action: 'UPDATE p SET id = 3 WHERE id = 1', + }); + expect(got).toBe('reject'); + }); + + // DIVERGENCE — PostgreSQL sets the child to its DEFAULT (2, a valid parent). + // pg-mem sets it to NULL. Pinned so Tier 0 code cannot rely on ON DELETE SET DEFAULT. + test('DIVERGENCE: pg-mem sets NULL on ON DELETE SET DEFAULT (PostgreSQL sets 2)', async () => { + const got = await probe({ + childFk: 'DEFAULT 2 REFERENCES p(id) ON DELETE SET DEFAULT', + action: 'DELETE FROM p WHERE id = 1', + }); + expect(got).toBe('null'); + }); +});