mirror of
https://github.com/block/buzz.git
synced 2026-11-15 13:51:12 +00:00
test(desktop): unmount every TerminalSubstrate tree before the window closes (#9440)
## Summary `desktop/src/features/terminal/TerminalSubstrate.test.mjs` can pass every test and then run the Desktop JS job out of memory several minutes later. This moves the React Testing Library unmount from `beforeEach` to `afterEach`, so no tree is still mounted when the jsdom window closes. Only the test file changes (6 insertions, 2 deletions). Commit (cherry-picked unchanged from a local flake-fix series): - `test(desktop): unmount every TerminalSubstrate tree before the window closes` ### CI sighting One sighting was found in the CI runs from 7 to 12 November 2026 that were searched: - 9 November 2026, "Desktop Domain / Desktop JS", run 42706004493 attempt 1 (on #9227; the rerun passed): - https://github.com/block/buzz/actions/runs/42706004493/job/129463488242 In that job the suite reported 6,950 of 6,951 passing. The log shows `An update to TerminalSubstrate inside a test was not wrapped in act(...)` at 21:54:33Z, then `FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory` at 22:02:17Z, and the file is reported as failed at file level (`test at src/features/terminal/TerminalSubstrate.test.mjs:1:1`) after 504.6 s. ### Root cause The file called `cleanup()` only in `beforeEach`, so the last test's tree was still mounted when `after(() => dom.window.close())` ran. If ThemeProvider's asynchronous palette update landed after that close, it re-rendered the substrate, the splash effect's palette guard passed, and `requestAnimationFrame` on the closed window started a fresh jsdom interval that nothing cleared. That 60 Hz loop repainted the banner into the test's recording `getContext` stub indefinitely, and `paintLog` grew until the heap limit. No individual test hangs, so a per-test timeout would not catch it. ### Fix `afterEach(() => cleanup?.())` unmounts every tree, including the last, before `after()` closes the window. The `beforeEach` call became redundant and is removed. No assertions change, and no timeouts, retries or sleeps are added. ### Related issue N/A (flake fix). No existing issue or PR for this failure was found. Three open PRs touch the same test file (#5057, #5485, #7777); none of them changes where `cleanup` runs. ### Testing Human testing is still required. This PR does not carry `buzz-review-completed`; the author has not yet run this test themselves on this branch. #### Verification of the fix (before this branch was split out) - The test file, 5 consecutive runs: 25 of 25 tests passed each time in about 6.8 s, with no `act` warning. - The CI out-of-memory failure was not reproduced with the real test file. Its mechanism was reproduced in a scratch harness with the same stubs: without cleanup before the window close, 3 of 3 runs left the palette unresolved at close, added 11,556 `paintLog` entries per 100 ms, kept one live timer, and never exited. With cleanup before the close, 3 of 3 runs added no entries, left no timers, and exited on their own. #### Local checks on this branch The push ran the repository's full pre-push hook set from a checkout of this branch's head (`7f4117340`), with no lanes excluded and without `--no-verify`. The branch is one commit behind `main`; that commit changes only mobile and docs files, none of them touched here. Ran and passed: - `branch-skew`, `push-head-scope` and `file-size-check` - `desktop-check` (`pnpm check`) - `desktop-typecheck` (`pnpm typecheck`) - `desktop-test` (`pnpm test`, the full desktop unit suite, which includes `TerminalSubstrate.test.mjs`) Did not run: - `rust-tests`, `desktop-tauri-checks` and `mobile-checks`. The hook selects lanes by changed path, and this branch changes no Rust, Tauri or mobile file. - Desktop builds and the Playwright E2E suites, which are CI-only. - `just ci`. - The 5-run repetition and the scratch harness above were not repeated on this branch. A green run does not prove the flake is gone: the failure needs the palette update to land after the window closes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: mtoohey <mtoohey@squareup.com> Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { after, before, beforeEach, test } from "node:test";
|
||||
import { after, afterEach, before, beforeEach, test } from "node:test";
|
||||
|
||||
import { JSDOM } from "jsdom";
|
||||
|
||||
@@ -67,8 +67,12 @@ before(async () => {
|
||||
});
|
||||
|
||||
after(() => dom.window.close());
|
||||
// Unmount after every test, not before the next one: a tree still mounted when
|
||||
// the window closes can be re-rendered by ThemeProvider's late palette update,
|
||||
// whose `requestAnimationFrame` on the closed window starts a jsdom interval
|
||||
// that nothing clears and that fills `paintLog` until the heap limit.
|
||||
afterEach(() => cleanup?.());
|
||||
beforeEach(() => {
|
||||
cleanup?.();
|
||||
reducedMotion = false;
|
||||
paintLog.length = 0;
|
||||
dom.window.localStorage.clear();
|
||||
|
||||
Reference in New Issue
Block a user