diff --git a/TEST_INTEGRITY.md b/TEST_INTEGRITY.md index 93386d67..a8c03581 100644 --- a/TEST_INTEGRITY.md +++ b/TEST_INTEGRITY.md @@ -509,3 +509,42 @@ test_integrity_note: did_relax_any_assertion: false if_true_explain_spec_basis: "" ``` + +## 2026-07-05 — todo-11b: un-quarantine dm-block-filter (hermetic subscription; no force-exit) + +```yaml +test_integrity_note: + change_type: ["flake_fix"] + scenarios: + - id: SCN-dm-block-filter-hermetic + given: "The 'integration with loaded user block list' test loads standard + legacy block lists and filters incoming DMs" + when: "manager.loadBlocks() runs against stubbed fetchListIncrementally, with the live block-list subscription stubbed out" + then: "Both blocked contacts (standard + legacy) are filtered (dmMessages === [ALLOWED]) AND the process opens NO real relay sockets, so node:test exits naturally with the correct exit code" + observable_outcomes: + - "manager.isBlocked(legacy) && manager.isBlocked(standard) === true; dmMessages length 1 (ALLOWED only)" + - "process.getActiveResourcesInfo() shows no TCPSocketWrap after loadBlocks; node exits 0 on pass, 1 on a forced failure (verified — failures NOT masked)" + determinism_controls: + - "stubbed nostrClient.fetchListIncrementally; truthy stub nostrClient.pool; no-op nostrClient.getSubscriptionManager; window.nostr.nip04.decrypt stub. All restored in finally. No network." + anti_cheat_rationale: + prevents: + - "retry/sleep-based flake masking" + - "over-mocking internal logic" + note: | + FLAKE_FIX, no assertion changed. The file PASSED all subtests then hung: loadBlocks() + opens a LIVE block-list subscription through nostrClient's real SimplePool aimed at the + test's fake relay URLs, leaking TCP sockets + reconnect timers that outlive the run so + node:test never exits (surfaced only once the runner was given a per-file timeout). + Diagnosed with process.getActiveResourcesInfo() — 4 TCPSocketWrap + reconnect Timeouts; + NOT the decrypt-retry timer (verified absent). Nulling the pool did not help because + loadBlocks calls ensurePool() (userBlocks.js:967) to re-create+connect a real pool when + pool is falsy. Fix keeps the test hermetic: a TRUTHY stub pool (so ensurePool is skipped) + + a no-op getSubscriptionManager (so the subscription opens nothing). The block-merge + logic under test still runs fully through the stubbed fetchListIncrementally. A force + `process.exit` in an after() hook was explicitly REJECTED after verifying it masks real + failures (process.exitCode is not set when the hook's timer fires — a deliberately broken + assertion still exited 0 under that approach). Under this fix, the forced-failure control + correctly exits 1. + relaxation: + did_relax_any_assertion: false + if_true_explain_spec_basis: "" +``` diff --git a/scripts/run-unit-tests.mjs b/scripts/run-unit-tests.mjs index 9ac36eb6..3457b798 100755 --- a/scripts/run-unit-tests.mjs +++ b/scripts/run-unit-tests.mjs @@ -13,9 +13,7 @@ process.env.NODE_ENV = "test"; // imported `node:test`, silently excluding ~20 bare-assert files; several were // failing on main, unnoticed). Each entry MUST cite why and link the triage item. // Remove an entry as soon as its file is fixed. See todo/TODO_2026-06-20_pre-launch.md todo-11b. -const QUARANTINE = new Map([ - ["tests/dm-block-filter.test.mjs", "HANG (post-completion) — all subtests PASS (TAP 'ok'), then the process hangs on a lingering handle from the imported app stack (nostrService/userBlocks/nostrClient/relayManager) so node:test never exits. Surfaced 2026-07-05 by running the full suite with UNIT_TEST_TIMEOUT_MS (pre-existing; it was never in this map). Needs a node:test-SAFE fix: close the leaked resource in an after() hook so node exits naturally — a force `process.exit` in after() is UNSAFE here (process.exitCode isn't set yet, verified, so it masks real failures). triage todo-11b"], -]); +const QUARANTINE = new Map([]); const rootDir = path.dirname(fileURLToPath(new URL("../package.json", import.meta.url))); const testsDir = path.join(rootDir, "tests"); diff --git a/tests/dm-block-filter.test.mjs b/tests/dm-block-filter.test.mjs index 0a6fbf6a..14afa235 100644 --- a/tests/dm-block-filter.test.mjs +++ b/tests/dm-block-filter.test.mjs @@ -194,6 +194,8 @@ describe("DM block/mute filtering", () => { : nostrClient.relays; const originalFetchListIncrementally = nostrClient.fetchListIncrementally; const originalRelayEntries = relayManager.getEntries(); + const originalPool = nostrClient.pool; + const originalGetSubscriptionManager = nostrClient.getSubscriptionManager; const relayUrls = ["wss://dm-block-regression.example"]; relayManager.setEntries( @@ -202,6 +204,22 @@ describe("DM block/mute filtering", () => { ); nostrClient.relays = relayUrls; + // Keep this test hermetic. Otherwise loadBlocks() opens a LIVE block-list + // subscription through nostrClient's real SimplePool aimed at the fake relay + // above, leaking TCP sockets + reconnect timers that outlive the test and + // hang the node:test runner AFTER every assertion has already passed. + // (1) A truthy stub pool stops loadBlocks from calling ensurePool(), which + // otherwise re-creates and connects a real pool when pool is falsy + // (userBlocks.js) — so nulling the pool alone is not enough. + // (2) A no-op subscription manager makes startBlockListSubscription() a + // no-op instead of a real relay REQ. + // The block MERGE under test still runs entirely through the stubbed + // fetchListIncrementally below. Both are restored in finally. + nostrClient.pool = { __mockForDmBlockFilterTest: true }; + nostrClient.getSubscriptionManager = () => ({ + subscribe: () => ({ close() {} }), + }); + if (typeof globalThis.window === "undefined") { globalThis.window = {}; } @@ -317,6 +335,8 @@ describe("DM block/mute filtering", () => { }); nostrClient.relays = originalRelays; nostrClient.fetchListIncrementally = originalFetchListIncrementally; + nostrClient.pool = originalPool; + nostrClient.getSubscriptionManager = originalGetSubscriptionManager; if (typeof globalThis.window === "undefined") { globalThis.window = {}; } diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index 9801799e..1a3b576a 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -1347,14 +1347,22 @@ test below was failing on `main` unnoticed). decrypt-timeout section installs a never-resolving nip44Decrypt that leaves a background decrypt-retry timer alive (manager.reset() doesn't cancel it). Applied the same `setTimeout(() => process.exit(0), 50)` exit after the final assertion. - - **All THREE #11b target files un-quarantined.** Running the full suite with a - per-file timeout then surfaced a FOURTH, pre-existing hang that was never in the - map: `tests/dm-block-filter.test.mjs` (node:test) — all subtests PASS, then it - hangs on a lingering app-stack handle so node:test never exits. Quarantined with a - triage note (needs a node:test-safe resource-close fix, NOT a force-exit — a force - `process.exit` in an after() hook was verified to mask failures because - process.exitCode isn't set yet). So the map has ONE entry (dm-block-filter), down - from the original two, with the newly-found file documented. + - `dm-block-filter` — ✅ DONE + UN-QUARANTINED 2026-07-05. A FOURTH, pre-existing hang + surfaced by running the full suite with a per-file timeout (it was never in the map, + so it had been silently hanging local `npm run test:unit`). All subtests PASS, then + node:test never exits. Root cause (diagnosed via process.getActiveResourcesInfo): + the "integration with loaded user block list" test's `manager.loadBlocks()` opens a + LIVE block-list subscription through nostrClient's real SimplePool aimed at the + test's fake relay URLs, leaking 4 TCP sockets + reconnect timers. NOT the + decrypt-retry timer (verified absent). Nulling the pool did NOT help — loadBlocks + calls `ensurePool()` (userBlocks.js:967) to re-create+connect a real pool when pool + is falsy. Fix (node:test-SAFE, no force-exit): make the test hermetic — (1) a TRUTHY + stub pool so ensurePool() is skipped, (2) a no-op getSubscriptionManager so the + subscription opens nothing; the block MERGE still runs via the stubbed + fetchListIncrementally. Verified: clean run exits 0 (# pass 12), AND a deliberately + broken assertion still exits 1 (# fail 1) — failures are NOT masked. See + TEST_INTEGRITY.md. + - **QUARANTINE map is now EMPTY** — every unit test file runs in CI. ### 12. Promotion: `unstable → beta` - [ ] After this batch soaks and the high-priority items land, promote `unstable → beta`