diff --git a/eslint.config.ts b/eslint.config.ts index 2505f4b..5287876 100644 --- a/eslint.config.ts +++ b/eslint.config.ts @@ -382,6 +382,194 @@ const browserDataBoundaryPlugin = { }, }; +/** + * A DOM element rendered by React carries `__reactFiber$*` / `__reactProps$*` + * as *own enumerable* properties. `node:assert` builds its `AssertionError` + * eagerly, running `util.inspect` over both operands with `depth: 1000`, + * `getters: true` and `maxArrayLength: Infinity`; the fiber graph re-expands + * once per traversal path, so inspecting one rendered element allocates + * gigabytes and the worker dies before any `AssertionError` is ever thrown. + * A genuine regression then reports as an OOM or an opaque timeout instead of + * a failed assertion. Measured on this repo's Asset Library heading: + * depth 6 = 1.3MB, depth 8 = 7.8MB, depth 10 = 36MB, depth 12 = 135MB. + * + * Equality assertions only inspect their operands on failure, so an unsafe + * comparison stays invisible while green and detonates the day the behaviour + * it guards regresses -- which is exactly when the diagnosis is needed. + * + * `expect` is not affected: vitest prints and diffs through pretty-format's + * DOM plugin, which reads tag/attributes/children and never touches the fiber. + * So the safe form is always an `expect` matcher -- `toHaveFocus()`, + * `not.toBeInTheDocument()`, `toBe(element)` -- and this rule only forbids + * handing a DOM element to `node:assert`. + */ +const TESTING_LIBRARY_QUERY = + /^(get|query|find)(All)?By(Role|Text|LabelText|PlaceholderText|AltText|Title|DisplayValue|TestId)$/u; +const domQueryMethods = new Set([ + "querySelector", + "querySelectorAll", + "getElementById", + "closest", +]); +const domElementProperties = new Set([ + "activeElement", + "parentElement", + "firstElementChild", + "lastElementChild", + "nextElementSibling", + "previousElementSibling", + "offsetParent", +]); +// Fail when the operands *differ*, so on failure at least one element is +// still there to be inspected. +const positiveAssertEqualities = new Set([ + "equal", + "strictEqual", + "deepEqual", + "deepStrictEqual", +]); +// Fail when the operands *match*. `assert.notEqual(element, null)` can only +// fail with `null` on both sides, so a nullish literal operand makes these +// safe; anything else leaves an element to inspect. +const negativeAssertEqualities = new Set([ + "notEqual", + "notStrictEqual", + "notDeepEqual", + "notDeepStrictEqual", +]); + +const noElementOperandEqualityRule: Rule.RuleModule = { + meta: { + type: "problem", + schema: [], + messages: { + unbounded: + "node:assert inspects both operands at depth 1000 to build its failure message, and a React-rendered element's __reactFiber$* graph exhausts the worker heap there, so the regression reports as an OOM instead of an assertion. Use an expect matcher instead -- expect(el).toHaveFocus(), expect(el).not.toBeInTheDocument(), expect(actual).toBe(expected) -- which prints DOM nodes through pretty-format's DOM plugin.", + }, + }, + create(context) { + const sourceCode = context.sourceCode; + + const unwrap = (input: any): any => { + let node = input; + while ( + node && + [ + "AwaitExpression", + "ChainExpression", + "TSAsExpression", + "TSNonNullExpression", + "TSSatisfiesExpression", + "TSTypeAssertion", + ].includes(node.type) + ) { + node = node.type === "AwaitExpression" ? node.argument : node.expression; + } + return node; + }; + + const memberName = (node: any): string | null => { + if (!node.computed && node.property?.type === "Identifier") { + return node.property.name; + } + if ( + node.computed && + (node.property?.type === "Literal" || + node.property?.type === "StringLiteral") && + typeof node.property.value === "string" + ) { + return node.property.value; + } + return null; + }; + + const resolveInit = (node: any): any => { + const scope = sourceCode.getScope(node); + let current: any = scope; + while (current) { + const variable = current.variables.find( + (entry: any) => entry.name === node.name, + ); + if (variable) { + const definition = variable.defs.at(-1); + return definition?.node?.type === "VariableDeclarator" + ? definition.node.init + : null; + } + current = current.upper; + } + return null; + }; + + const isElementValued = (input: any, seen = new Set()): boolean => { + const node = unwrap(input); + if (!node || seen.has(node)) return false; + seen.add(node); + if (node.type === "MemberExpression") { + const name = memberName(node); + return name !== null && domElementProperties.has(name); + } + if (node.type === "CallExpression") { + const callee = unwrap(node.callee); + if (callee?.type !== "MemberExpression") return false; + const name = memberName(callee); + return ( + name !== null && + (TESTING_LIBRARY_QUERY.test(name) || domQueryMethods.has(name)) + ); + } + if (node.type === "Identifier") { + return isElementValued(resolveInit(node), seen); + } + if (node.type === "ConditionalExpression") { + return ( + isElementValued(node.consequent, seen) || + isElementValued(node.alternate, seen) + ); + } + return false; + }; + + const isNullish = (input: any): boolean => { + const node = unwrap(input); + if (!node) return false; + return ( + (node.type === "Literal" && node.value === null) || + (node.type === "Identifier" && node.name === "undefined") + ); + }; + + return { + CallExpression(node: any) { + const callee = unwrap(node.callee); + if (callee?.type !== "MemberExpression") return; + const object = unwrap(callee.object); + if (object?.type !== "Identifier" || object.name !== "assert") return; + + const name = memberName(callee); + if (name === null) return; + const positive = positiveAssertEqualities.has(name); + if (!positive && !negativeAssertEqualities.has(name)) return; + + const operands = (node.arguments ?? []).slice(0, 2); + if (!positive && operands.some((argument: any) => isNullish(argument))) { + return; + } + const operand = operands.find((argument: any) => + isElementValued(argument), + ); + if (operand) context.report({ node: operand, messageId: "unbounded" }); + }, + }; + }, +}; + +const testAssertionBoundaryPlugin = { + rules: { + "no-element-operand-equality": noElementOperandEqualityRule, + }, +}; + const commonLanguageOptions = { ecmaVersion: "latest", sourceType: "module", @@ -811,6 +999,12 @@ export default [ ...globals.node, }, }, + plugins: { + "test-assertion-boundary": testAssertionBoundaryPlugin, + }, + rules: { + "test-assertion-boundary/no-element-operand-equality": "error", + }, }, { files: [`tests/support/browser/**/*.${sourceExtensions}`], diff --git a/tests/features/tech-log/asset-library.test.tsx b/tests/features/tech-log/asset-library.test.tsx index a0bf8b6..ff497eb 100644 --- a/tests/features/tech-log/asset-library.test.tsx +++ b/tests/features/tech-log/asset-library.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment jsdom import assert from "node:assert/strict"; -import { test } from "vitest"; +import { expect, test } from "vitest"; import { act, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; @@ -132,7 +132,7 @@ test("shows archive instead of delete for an asset in use", async () => { await user.click(await screen.findByRole("button", { name: "boundary" })); assert.ok(await screen.findByRole("button", { name: "보관" })); - assert.equal(screen.queryByRole("button", { name: "삭제" }), null); + expect(screen.queryByRole("button", { name: "삭제" })).not.toBeInTheDocument(); }); test("surfaces ASSET_IN_USE when the server rejects a delete", async () => { @@ -216,7 +216,7 @@ test("removes the asset and restores focus to the page heading, not the detached const options = deleteCalls[0].options as { idempotencyKey?: unknown }; assert.equal(typeof options.idempotencyKey, "string"); assert.ok((options.idempotencyKey as string).length > 0); - assert.equal(screen.queryByRole("button", { name: "boundary" }), null); + expect(screen.queryByRole("button", { name: "boundary" })).not.toBeInTheDocument(); // `findByText` above already settled on the post-delete render -- the // `queueMicrotask`-scheduled focus call has necessarily run by then, since @@ -224,8 +224,16 @@ test("removes the asset and restores focus to the page heading, not the detached // (its polling uses `setTimeout`/`MutationObserver`), and microtasks always // drain before the next macrotask runs. So this asserts directly rather // than polling. + // + // `toHaveFocus`, never `assert.equal(document.activeElement, heading)`: + // node:assert builds an AssertionError by running `util.inspect` over both + // operands at `depth: 1000`, and a React-rendered element carries + // `__reactFiber$*` own properties whose graph re-expands per traversal + // path. Inspecting one costs gigabytes, so the regression this test guards + // used to kill the worker instead of reporting itself. jest-dom prints DOM + // nodes through pretty-format's DOM plugin, which never touches the fiber. const heading = screen.getByRole("heading", { name: "Asset", level: 1 }); - assert.equal(document.activeElement, heading); + expect(heading).toHaveFocus(); }); // --- Alignment follow-up, item 1: the management screen gets search and paging --- @@ -280,7 +288,7 @@ test("searches assets with the typed query and announces the new result count", await user.click(screen.getByRole("button", { name: "검색" })); assert.ok(await screen.findByRole("button", { name: "older-diagram" })); - assert.equal(screen.queryByRole("button", { name: "newer-diagram" }), null); + expect(screen.queryByRole("button", { name: "newer-diagram" })).not.toBeInTheDocument(); assert.equal(calls.at(-1)?.q, "older"); // The result count lives in a `role="status"` region so a screen reader is // told the list changed -- a silently re-rendered list is the failure mode @@ -324,7 +332,7 @@ test("loads the next page with the server cursor and appends it to the list", as assert.equal(calls.at(-1)?.cursor, "cursor-1"); // Exhausted paging removes the control rather than leaving a button that // does nothing. - assert.equal(screen.queryByRole("button", { name: "더 보기" }), null); + expect(screen.queryByRole("button", { name: "더 보기" })).not.toBeInTheDocument(); }); test("더 보기 moves focus to the first newly loaded asset instead of stranding it on the removed control", async () => { @@ -342,21 +350,14 @@ test("더 보기 moves focus to the first newly loaded asset instead of strandin await user.click(screen.getByRole("button", { name: "더 보기" })); const appended = await screen.findByRole("button", { name: "focus-page-two" }); - // Compared as a boolean, not as two nodes: `assert.equal(node, node)` builds - // its failure message by inspecting both jsdom elements, and that traversal - // is heavy enough to take the worker down instead of reporting the failure. - assert.equal( - document.activeElement === appended, - true, - `focus was on ${document.activeElement?.textContent ?? "nothing"}`, - ); + expect(appended).toHaveFocus(); }); test("offers no 더 보기 control when the server reports no next cursor", async () => { render(); await screen.findByRole("button", { name: "boundary" }); - assert.equal(screen.queryByRole("button", { name: "더 보기" }), null); + expect(screen.queryByRole("button", { name: "더 보기" })).not.toBeInTheDocument(); }); test("a new search restarts paging from the first page instead of carrying the old cursor", async () => { @@ -380,8 +381,8 @@ test("a new search restarts paging from the first page instead of carrying the o await user.click(screen.getByRole("button", { name: "검색" })); assert.ok(await screen.findByRole("button", { name: "reset-found" })); - assert.equal(screen.queryByRole("button", { name: "reset-page-one" }), null); - assert.equal(screen.queryByRole("button", { name: "reset-page-two" }), null); + expect(screen.queryByRole("button", { name: "reset-page-one" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "reset-page-two" })).not.toBeInTheDocument(); assert.equal(calls.at(-1)?.cursor, undefined); }); @@ -438,7 +439,7 @@ test("a slow earlier search never overwrites the newer search's results", async pending[1]!.settle({ items: [slow], nextCursor: null }); }); - assert.equal(screen.queryByRole("button", { name: "race-slow" }), null); + expect(screen.queryByRole("button", { name: "race-slow" })).not.toBeInTheDocument(); assert.ok(screen.getByRole("button", { name: "race-fast" })); }); @@ -454,5 +455,5 @@ test("a search that matches nothing says so instead of claiming no asset was eve await user.click(screen.getByRole("button", { name: "검색" })); assert.ok(await screen.findByText("조건에 맞는 Asset이 없습니다")); - assert.equal(screen.queryByText("등록된 Asset이 없습니다"), null); + expect(screen.queryByText("등록된 Asset이 없습니다")).not.toBeInTheDocument(); }); diff --git a/tests/features/tech-log/asset-picker.test.tsx b/tests/features/tech-log/asset-picker.test.tsx index db96d8c..f4c01f7 100644 --- a/tests/features/tech-log/asset-picker.test.tsx +++ b/tests/features/tech-log/asset-picker.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment jsdom import assert from "node:assert/strict"; -import { afterAll, beforeAll, test } from "vitest"; +import { afterAll, beforeAll, expect, test } from "vitest"; import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { MemoryRouter } from "react-router-dom"; @@ -143,7 +143,7 @@ test("does not offer a QUARANTINED asset for insertion", async () => { render( {}} />); await screen.findByRole("button", { name: /fetch-strategy-boundary/ }); - assert.equal(screen.queryByRole("button", { name: /unsafe/ }), null); + expect(screen.queryByRole("button", { name: /unsafe/ })).not.toBeInTheDocument(); }); test("reports the loaded Asset list once listAssets resolves", async () => { @@ -398,14 +398,19 @@ test("focuses the file input on open, traps Tab inside the dialog, and calls onC const altInput = screen.getByLabelText("대체 텍스트"); const uploadButton = screen.getByRole("button", { name: "업로드" }); const closeButton = screen.getByRole("button", { name: "닫기" }); - assert.equal(document.activeElement, input); + // `toHaveFocus`, never `assert.equal(document.activeElement, input)`: + // node:assert inspects both operands at `depth: 1000` to build its failure + // message, and a React-rendered element's `__reactFiber$*` graph re-expands + // per traversal path, so inspecting one exhausts the heap and kills the + // worker instead of reporting the regression. + expect(input).toHaveFocus(); // Final fix wave, item 2: the trap now has to hold across the two metadata // controls the dialog gained, and still wrap from the last back to the first. input.focus(); for (const expected of [decorativeBox, altInput, closeButton, uploadButton, input]) { await user.tab(); - assert.equal(document.activeElement, expected); + expect(expected).toHaveFocus(); } await user.click(closeButton); @@ -457,7 +462,7 @@ test("inserts the directive at the saved cursor position and the live preview re await user.click(screen.getByRole("tab", { name: "즉시 미리보기" })); const panel = screen.getByRole("tabpanel", { name: "즉시 미리보기" }); - assert.equal(within(panel).queryByRole("alert"), null); + expect(within(panel).queryByRole("alert")).not.toBeInTheDocument(); // `zoom: true` (DIAGRAM kind) renders the figure's image twice -- once as // the zoom trigger, once inside the (closed) zoom dialog -- both share the // same alt text, so assert on the first (the visible trigger figure). @@ -1299,7 +1304,7 @@ test("Instant Preview renders the same asset the gate and the descriptor picked [newer, older], ); - assert.equal(screen.queryByRole("alert"), null); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); assert.equal( screen.getByAltText("중복 키 그림").getAttribute("src"), "/media/dup-pixels-newer.svg", @@ -1629,7 +1634,7 @@ test("the Picker shows a first page before anything is typed, then narrows to th await user.click(screen.getByRole("button", { name: "검색" })); await waitFor(() => - assert.equal(screen.queryByRole("button", { name: /inserted-diagram/ }), null), + expect(screen.queryByRole("button", { name: /inserted-diagram/ })).not.toBeInTheDocument(), ); assert.ok(screen.getByRole("button", { name: /other-diagram/ })); assert.equal(calls.at(-1)?.q, "other"); @@ -1680,7 +1685,7 @@ test("a QUARANTINED asset the server wrongly returns for a query is still not of await waitFor(() => assert.ok(screen.getByRole("button", { name: /inserted-diagram/ })), ); - assert.equal(screen.queryByRole("button", { name: /unsafe-diagram/ }), null); + expect(screen.queryByRole("button", { name: /unsafe-diagram/ })).not.toBeInTheDocument(); }); test("the Picker reports search results too, so the catalog grows with every query", async () => { @@ -1753,7 +1758,7 @@ test("a slow earlier Picker search never overwrites the newer query's results", pending[1]!.settle({ items: [PICKER_A], nextCursor: null }); }); - assert.equal(screen.queryByRole("button", { name: /inserted-diagram/ }), null); + expect(screen.queryByRole("button", { name: /inserted-diagram/ })).not.toBeInTheDocument(); assert.ok(screen.getByRole("button", { name: /other-diagram/ })); }); @@ -1792,7 +1797,7 @@ test("a Picker search never drops an already-inserted asset out of Instant Previ await user.type(screen.getByLabelText("Asset 검색"), "other"); await user.click(screen.getByRole("button", { name: "검색" })); await waitFor(() => - assert.equal(screen.queryByRole("button", { name: /inserted-diagram/ }), null), + expect(screen.queryByRole("button", { name: /inserted-diagram/ })).not.toBeInTheDocument(), ); await user.click(screen.getByRole("tab", { name: "즉시 미리보기" })); diff --git a/vitest.config.ts b/vitest.config.ts index 5358cb1..b146aa9 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -8,6 +8,16 @@ export default defineConfig({ clearMocks: true, mockReset: true, testTimeout: 10_000, + // A runaway serialization inside a worker (the classic case: an assertion + // library inspecting a React-rendered DOM node, whose `__reactFiber$*` + // graph re-expands per traversal path) allocates until the machine's OOM + // killer takes the whole run down -- 29.5GB and ~40s here, reported as a + // bare "worker exited" with no failing test named. Capping the worker's + // old-space turns that into a `Reached heap limit` abort in ~3s, next to + // the file that caused it. The ceiling is chosen well above real demand: + // the heaviest suite (`tests/unit`, 126 files) peaks near 1.3GB RSS, of + // which old-space is only a fraction. + execArgv: ["--max-old-space-size=2048"], exclude: [ ...configDefaults.exclude, ".tmp/**",