Public: anyone can read this; the crew can change it.
Make the N+1 query-count tests deterministic (surfaces-adversarial) #1845
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/1799-make-the-n-1-query-count-tests-determini → main 1 commit fork
2 files
changed +81
−18
test/helpers/statements.ts added +78
| @@ -0,0 +1,78 @@ | ||
| 1 | /** | |
| 2 | * Count the SQL statements a block of work runs, the same way every time. | |
| 3 | * | |
| 4 | * `db.query` is the one door every read and write goes through, so wrapping it | |
| 5 | * counts statements without threading a counter through the code under test. | |
| 6 | * But a server in the same process shares that door with everything else it | |
| 7 | * does: the standing sweep, an earlier request's fire-and-forget work, an | |
| 8 | * event handler that lands late. That noise is only ever *added* to a count, | |
| 9 | * and it is the whole reason two runs of one test disagreed (108 vs 158, | |
| 10 | * #1799). So: | |
| 11 | * | |
| 12 | * - `quiet` waits until nothing has touched the database for a moment, so a | |
| 13 | * measurement starts from rest rather than in the tail of the last step; | |
| 14 | * - `statements` measures the work several times and keeps the smallest. A | |
| 15 | * real N+1 costs more in every run, so the minimum still shows it; noise | |
| 16 | * costs more in some runs, so the minimum drops it. | |
| 17 | * | |
| 18 | * `work` must be repeatable: a request for the same page, not a mutation. | |
| 19 | */ | |
| 20 | ||
| 21 | import type { Database } from "bun:sqlite"; | |
| 22 | ||
| 23 | const QUIET_MS = 40; | |
| 24 | const GIVE_UP_MS = 2_000; | |
| 25 | const RUNS = 5; | |
| 26 | ||
| 27 | type Door = { query: unknown }; | |
| 28 | ||
| 29 | /** Run `work` once with the statement counter on; return what it cost. */ | |
| 30 | async function once( | |
| 31 | db: Database, | |
| 32 | work: () => Promise<void> | void, | |
| 33 | ): Promise<number> { | |
| 34 | const door = db as unknown as Door; | |
| 35 | const original = db.query.bind(db); | |
| 36 | let count = 0; | |
| 37 | door.query = (sql: string) => { | |
| 38 | count++; | |
| 39 | return original(sql); | |
| 40 | }; | |
| 41 | try { | |
| 42 | await work(); | |
| 43 | } finally { | |
| 44 | door.query = original; | |
| 45 | } | |
| 46 | return count; | |
| 47 | } | |
| 48 | ||
| 49 | /** Resolve once no statement has run for `QUIET_MS` (or after `GIVE_UP_MS`). */ | |
| 50 | export async function quiet(db: Database): Promise<void> { | |
| 51 | const door = db as unknown as Door; | |
| 52 | const original = db.query.bind(db); | |
| 53 | let last = Date.now(); | |
| 54 | door.query = (sql: string) => { | |
| 55 | last = Date.now(); | |
| 56 | return original(sql); | |
| 57 | }; | |
| 58 | try { | |
| 59 | const started = Date.now(); | |
| 60 | while (Date.now() - last < QUIET_MS && Date.now() - started < GIVE_UP_MS) | |
| 61 | await new Promise((resolve) => setTimeout(resolve, QUIET_MS / 4)); | |
| 62 | } finally { | |
| 63 | door.query = original; | |
| 64 | } | |
| 65 | } | |
| 66 | ||
| 67 | /** The fewest statements `work` costs over several runs from rest. */ | |
| 68 | export async function statements( | |
| 69 | db: Database, | |
| 70 | work: () => Promise<void> | void, | |
| 71 | ): Promise<number> { | |
| 72 | let fewest = Number.POSITIVE_INFINITY; | |
| 73 | for (let run = 0; run < RUNS; run++) { | |
| 74 | await quiet(db); | |
| 75 | fewest = Math.min(fewest, await once(db, work)); | |
| 76 | } | |
| 77 | return fewest; | |
| 78 | } |
test/surfaces-adversarial.test.ts +3 −18
| @@ -14,7 +14,6 @@ | ||
| 14 | 14 | */ |
| 15 | 15 | |
| 16 | 16 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; |
| 17 | import type { Database } from "bun:sqlite"; | |
| 18 | 17 | import { mkdtemp, rm } from "node:fs/promises"; |
| 19 | 18 | import { tmpdir } from "node:os"; |
| 20 | 19 | import { join } from "node:path"; |
| @@ -36,26 +35,11 @@ import { createForgeApp } from "../src/tui/forge.ts"; | ||
| 36 | 35 | import type { Session, TuiIdentity } from "../src/tui/session.ts"; |
| 37 | 36 | import type { App as TuiApp, Screen } from "../src/tui/app/app.ts"; |
| 38 | 37 | import { headlessTerminal } from "../src/tui/term/terminal.ts"; |
| 38 | import { statements } from "./helpers/statements.ts"; | |
| 39 | 39 | import { settleApp } from "./helpers/tui.ts"; |
| 40 | 40 | |
| 41 | 41 | quietLogs(); |
| 42 | 42 | |
| 43 | /** Count the SQL statements a block of work runs — mirrors test/counts.test.ts. */ | |
| 44 | async function statements(db: Database, work: () => Promise<void> | void) { | |
| 45 | const original = db.query.bind(db); | |
| 46 | let count = 0; | |
| 47 | (db as unknown as { query: unknown }).query = (sql: string) => { | |
| 48 | count++; | |
| 49 | return original(sql); | |
| 50 | }; | |
| 51 | try { | |
| 52 | await work(); | |
| 53 | } finally { | |
| 54 | (db as unknown as { query: unknown }).query = original; | |
| 55 | } | |
| 56 | return count; | |
| 57 | } | |
| 58 | ||
| 59 | 43 | const words = (html: string): string => html.replace(/\s+/g, " "); |
| 60 | 44 | |
| 61 | 45 | // ============================================================================ |
| @@ -116,7 +100,8 @@ describe("surfaces adversarial: web", () => { | ||
| 116 | 100 | |
| 117 | 101 | beforeAll(async () => { |
| 118 | 102 | dataDir = await mkdtemp(join(tmpdir(), "forge-surfaces-adv-")); |
| 119 | app = createApp({ dataDir, port: 0 }); | |
| 103 | // The standing sweep queries on a timer; the N+1 counts are of requests alone. | |
| 104 | app = createApp({ dataDir, port: 0, standingSweepMs: 3_600_000 }); | |
| 120 | 105 | servers = await serve(app); |
| 121 | 106 | base = servers.url; |
| 122 | 107 | alice = await signUp("alice", alicePassword); // first account: the admin |
new above
offline