diff --git a/AGENTS.md b/AGENTS.md index 2cc2f883..e002b2b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -435,12 +435,16 @@ free to run a full `local:gate`). Raising a budget nobody chose hides no race. tool can be asked to resolve; assert at runtime when it is about behavior. - **A budget is stated, which is not the same as raised.** Testing Library's `asyncUtilTimeout` is pinned at its own default in both web setup files - because raising it was *measured worse*: a wait that is meant to expire — - a test asserting something never appears, a poll allowed to run out — spends - its whole budget on the happy path, so a raise is a proportional cost on - exactly those tests and starves the worker pool. Measure before raising a - budget that a passing test can spend in full; the call-site comment records - the three-arm run. + because raising it was *measured* and found to buy nothing: #2323's + three-arm run went red at 5000, and #2335's interleaved re-run on a leased + machine found no web test that lets a Testing Library wait expire on its + passing path and no difference between 1000 and 5000 beyond the guard + refusing the raised value. The hazard the rule guards against is still + real: a wait that is meant to expire — a test asserting something never + appears, a poll allowed to run out — spends its whole budget on the happy + path, so a raise is a proportional cost on exactly those tests. Measure + before raising a budget that a passing test can spend in full; the + call-site comment records both runs. - **An inner budget must be strictly smaller than the budget enclosing it.** `waitFor({ timeout: N })` inside a test whose own budget is `N` can never win: the test expires first and reports a timeout naming neither the wait nor its diff --git a/clients/web/src/test/asyncUtilTimeout.test.ts b/clients/web/src/test/asyncUtilTimeout.test.ts index c7817328..e13a8cc6 100644 --- a/clients/web/src/test/asyncUtilTimeout.test.ts +++ b/clients/web/src/test/asyncUtilTimeout.test.ts @@ -22,11 +22,15 @@ import { getConfig } from "@testing-library/react"; * * A different unrelated set failed each time and every one passed in about a * second in isolation, which is the shape of CPU starvation rather than of a - * slow assertion. The mechanism: `asyncUtilTimeout` is not only a ceiling — any - * wait that is *meant* to expire spends the whole budget on the happy path, so - * a 5x raise is a 5x cost on exactly those tests, which saturates the worker - * pool and starves tests that were never slow. The tests that assert absence - * that way are the real defect and are #2335; revisit this when they are gone. + * slow assertion. The mechanism proposed at the time — a wait that is *meant* + * to expire spends the whole budget on the happy path, so a 5x raise is a 5x + * cost on those tests — was checked on #2335 and does not hold here: no web + * test lets a Testing Library wait expire on its passing path, and the same + * three-arm protocol re-run interleaved on a leased machine showed no failure + * in any arm beyond this very assertion refusing the raised value, and 0 tests + * at or above 5000ms with the raise in place. Raising + * the budget buys nothing, so the pin stays at the default until a measurement + * says otherwise. `setup.ts` carries the full record. * * ⚠️ This asserts the EFFECTIVE value at runtime rather than checking that * `setup.ts` contains a `configure()` call. An earlier revision of #2334 did diff --git a/clients/web/src/test/setup.ts b/clients/web/src/test/setup.ts index 344c0ee2..dae9776c 100644 --- a/clients/web/src/test/setup.ts +++ b/clients/web/src/test/setup.ts @@ -21,17 +21,28 @@ import { cleanup, configure } from "@testing-library/react"; // // The failures were a different, unrelated set each time and every one passed // in about a second in isolation, which is the shape of CPU starvation rather -// than of a slow assertion. The mechanism is that `asyncUtilTimeout` is not -// only a ceiling: any wait that is *meant* to expire — a test asserting -// something never appears, and anything that lets a poll run out — spends the -// whole budget on the happy path, so a 5x raise is a 5x cost on those tests, -// which saturates the worker pool and starves tests that were never slow. +// than of a slow assertion. The mechanism proposed at the time was that +// `asyncUtilTimeout` is not only a ceiling: a wait that is *meant* to expire — +// a test asserting something never appears, a poll allowed to run out — spends +// the whole budget on the happy path, so a 5x raise would be a 5x cost on +// those tests, saturating the worker pool. +// +// #2335 went looking for those tests and found none. Zero hits for every +// expiring-wait shape against Testing Library across the unit, `core/`, +// integration and story files; no test at or above 2000ms in a full unit run; +// and the same three-arm protocol re-run interleaved on a leased machine +// (1000 / 5000 / reverted, three rounds) gave 0 failures in every arm — the +// one red test in each 5000 arm being `asyncUtilTimeout.test.ts` refusing the +// raised value, which is the guard doing its job — and 0 tests at or above +// 5000ms with the raise in place. Nothing on the passing +// path spends this budget, so raising it buys nothing — and the #2323 reds +// read as ambient load the arms did not share (an inference from that data, +// not a re-measurement of the original run). // // So the general rule this repo now follows — a budget must be a value someone -// chose — is satisfied by choosing it, not by raising it. Raising it is what -// #2323's own "do not raise" list would have said had the measurement been -// taken first. The tests that assert absence by letting a wait expire are the -// real defect and are tracked on #2335; when they are gone, revisit this. +// chose — is satisfied by choosing it, not by raising it. The pin stays at the +// default until a measurement says otherwise; `asyncUtilTimeout.test.ts` +// asserts the effective value, so moving it is deliberate by construction. configure({ asyncUtilTimeout: 1000 }); // Node 22+ exposes an experimental `localStorage` placeholder that overrides