From a40518133c161c2c81189b6d6523c6b95ca70c38 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 20:16:42 +0000 Subject: [PATCH 1/2] test(plugin-tree): split the contractEnvelope-6839 waits by expected outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file handed all three cases one wait — the table wrapper's mere presence — and read `tbody tr` the instant it passed. That testid is a MOUNT signal, but the rows arrive a commit later: `ObjectTree` keeps expansion in a `useState>(new Set())` mirror that a `useEffect` re-seeds from the forest, so the commit that first paints the table still carries the empty mirror and draws the root without its child. Probed on this fixture the DOM sequence is `loading -> table:1rows -> table:2rows`, and the old wait's first passing state was `table:1rows`, yielding 1 — the CI red, `expected 1 to be 2`. The positive arms now wait for the DESCENDANT row, the row the mirror gates, and assert the drawn shape plus the root toggle reading `Collapse`, so the pin is the seeded-open hierarchy rather than an eventual count. The refusal arm takes a settled read anchored on the "No records" panel, which the tree renders only after `loading` flips false, so it cannot pass by timing out on an absence. No timeout was raised, no assertion loosened. Part of objectui#6839 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .../ObjectTree.contractEnvelope-6839.test.tsx | 203 ++++++++++++++++-- 1 file changed, 183 insertions(+), 20 deletions(-) diff --git a/packages/plugin-tree/src/ObjectTree.contractEnvelope-6839.test.tsx b/packages/plugin-tree/src/ObjectTree.contractEnvelope-6839.test.tsx index 03a74cec1f..c896254303 100644 --- a/packages/plugin-tree/src/ObjectTree.contractEnvelope-6839.test.tsx +++ b/packages/plugin-tree/src/ObjectTree.contractEnvelope-6839.test.tsx @@ -28,10 +28,64 @@ * ⚠️ The refusal case is ALSO satisfied by an `extractRecords` that returns * `[]` for everything — an implementation strictly worse than the bug. The * `data` and bare-array cases refuse it: same rows, same mount. + * + * ## ⭐ Why the two outcomes wait DIFFERENTLY + * + * This file used to hand all three cases ONE wait, and it went red on `main` + * on PRs that cannot reach `plugin-tree` at all — `expected 1 to be 2`. + * + * That wait was `container.querySelector('[data-testid="object-tree"]') ?? + * queryByText('No records')`, and the row count was read the instant it passed. + * The testid is on the TABLE WRAPPER, so it is a MOUNT signal, not a rows + * signal — and the rows this file counts arrive at a LATER commit than the + * table does. + * + * MEASURED, the mechanism is one race, in `ObjectTree` itself: expansion is a + * `useState>(new Set())` MIRROR that a `useEffect` keyed on + * `[roots, defaultExpandedDepth]` re-seeds from the forest. Rows are then + * `flattenVisible(roots, expanded)`. So when `find()`'s rows land, the commit + * that first paints the table still carries the PREVIOUS (empty) mirror: the + * root draws, its child does not, and `tbody tr` is 1. The mirror is seeded in + * the passive effect that follows, and a second commit takes it to 2. Probed + * on this file's own fixture, the DOM sequence is exactly + * `loading → table:1rows → table:2rows`, and the OLD wait's first passing state + * was `table:1rows`, where it yielded 1 — the CI failure, by construction and + * not by luck. Which of the two commits the read lands on is decided by machine + * load, which is why it was green locally and red on a saturated shard. + * + * ⚠️ NOT the same shape as the `plugin-kanban` twin (objectui#8532 / PR #8533), + * although it is the same family. That board had TWO independent races — + * `React.lazy(() => import('./KanbanImpl'))` chunk reveal AND a prop-mirrored + * `boardColumns` — and its symptom was `expected +0 to be 2`, nothing drawn at + * all. `plugin-tree` has NO lazy boundary anywhere in its source (`index.tsx` + * imports `./ObjectTree` eagerly), so only the mirrored-state half transfers, + * and the symptom is a PARTIAL draw: the root without its child. + * + * The two outcomes therefore no longer share a wait: + * + * - the POSITIVE arms wait FOR the DESCENDANT row — the row that only exists + * once the mirror has been seeded. That is the condition the race is + * about, so waiting on it is what makes these arms immune to which commit + * the read lands on. It is NOT a wider window on the same race, which is + * all a raised timeout would have bought. They then assert the drawn SHAPE + * (label + depth per row) plus the root's toggle reading `Collapse`, so the + * pin is the seeded-open hierarchy and not merely "eventually 2 rows" — a + * count alone is also satisfied by a tree that flattens everything. + * - the REFUSAL arm cannot wait for an absence, so it takes a SETTLED read + * anchored on something that DOES appear in that scenario: the tree's own + * "No records" panel, which `ObjectTree` renders only once `loading` has + * flipped false. Probed on the `records` fixture the sequence is + * `loading → empty-state` with no table at any point, and the panel is + * absent while loading — so this arm cannot pass by timing out on an + * absence, which is the failure mode every absence-shaped pin has. + * + * ⛔ Do not fold these back into one wait, and ⛔ do not "fix" a future red here + * with a longer timeout: the failure was never slowness, it was reading a + * signal that does not carry the answer. */ import React from 'react'; -import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { render, waitFor, cleanup, within, act } from '@testing-library/react'; import { describe, it, expect, vi, afterEach } from 'vitest'; import { ObjectTree } from './ObjectTree'; @@ -66,21 +120,87 @@ const asData: Envelope = (rows) => ({ data: rows, total: rows.length }); const asBareArray: Envelope = (rows) => rows; const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); +/** One row the forest actually painted, as the tree-grid describes it. */ +interface DrawnRow { + readonly label: string; + readonly depth: number; +} + /** - * Mount the tree over a `find()` answering `envelope`, and hand back what it - * settled on: the number of rows it drew, or `'empty-state'` when it drew its - * own "No records" panel instead. + * Everything this file reads off a settled tree. * - * The two outcomes are DIFFERENT DOM, not a count of 0 — `ObjectTree` returns - * early on an empty forest and never mounts the table. Reporting which one - * happened is what keeps a refused envelope distinguishable from a mount that - * never rendered. + * `tableDrawn` and `emptyPanelDrawn` are carried SEPARATELY rather than + * collapsed into a row count: `ObjectTree` returns early on an empty forest and + * never mounts the table, so a refused envelope and a mount that never rendered + * are DIFFERENT DOM, not both "zero rows". Reporting which one happened is what + * keeps them distinguishable. + */ +interface Settled { + readonly rows: readonly DrawnRow[]; + readonly tableDrawn: boolean; + readonly emptyPanelDrawn: boolean; + /** The root row's toggle, by accessible name — the mirror's state, named. */ + readonly rootToggle: string | null; +} + +/** The rows the tree-grid has painted, in document order. */ +function drawnRows(container: HTMLElement): DrawnRow[] { + return Array.from( + container.querySelectorAll('tbody tr[data-testid="object-tree-row"]'), + ).map((tr) => ({ + label: (tr.querySelector('td')?.textContent ?? '').trim(), + depth: Number(tr.getAttribute('data-depth')), + })); +} + +/** + * Has the tree drawn its own "No records" panel? + * + * ⚠️ `queryAllByText`, not `queryByText`: the singular form THROWS on multiple + * matches as well as answering `null` on none, so it cannot express "how many" + * without the throw becoming the result. + */ +function emptyPanelDrawn(container: HTMLElement): boolean { + return within(container).queryAllByText('No records').length > 0; +} + +function readSettled(container: HTMLElement): Settled { + const root = container.querySelector('tbody tr[data-depth="0"]'); + return { + rows: drawnRows(container), + tableDrawn: container.querySelector('[data-testid="object-tree"]') !== null, + emptyPanelDrawn: emptyPanelDrawn(container), + rootToggle: root?.querySelector('button')?.getAttribute('aria-label') ?? null, + }; +} + +/** + * What a case expects the tree to settle on — which is also what decides HOW it + * waits. See this file's header. + */ +type Outcome = + /** Wait FOR that shape. `because` is carried into the timeout message. */ + | { readonly draws: readonly DrawnRow[]; readonly because: string } + /** No absence to wait for: anchor on the empty panel, settle, then read. */ + | { readonly refuses: true }; + +const REFUSES: Outcome = { refuses: true }; + +/** + * Mount the tree over a `find()` answering `envelope`, and hand back what it + * settled on once `outcome` says it has settled. * * ⛔ Call ONCE per case, never inside a `waitFor` predicate (objectui#7802): * it renders, and `waitFor` re-runs its callback on DOM mutations, so a * predicate that renders feeds itself and leaks a container div per run. + * + * ⚠️ The predicates BELOW are inside `waitFor` on purpose and stay sound under + * that same rule: `drawnRows` and `emptyPanelDrawn` are pure reads of a + * container that is already mounted. They mount nothing, so re-running them on + * a DOM mutation is free — which is exactly the property this helper itself + * does not have. */ -async function settledOn(envelope: Envelope): Promise { +async function settledOn(envelope: Envelope, outcome: Outcome): Promise { const find = vi.fn(async () => envelope(ROWS)); const ds: any = { find, @@ -103,17 +223,38 @@ async function settledOn(envelope: Envelope): Promise { // touches no DOM. Without it "no rows" is satisfied by the mount's initial // empty state, which every arm renders identically. await find.mock.results[0].value; + + if ('refuses' in outcome) { + // A refusal has no arrival to wait for, so this is a SETTLED read built + // from the two things that CAN be observed: `find` has answered (above), + // and the tree has drawn the "No records" panel — a node it renders only + // after `loading` flips false, so it is a COMPLETION ANCHOR and not the + // mere passage of time. `act` then flushes what React still had queued; it + // is the opposite of widening a timeout. + await waitFor(() => + expect( + emptyPanelDrawn(container), + 'the tree must have drawn its own "No records" panel before it is read — without that anchor an absence assertion passes by timing out', + ).toBe(true), + ); + await act(async () => {}); + return readSettled(container); + } + + // Wait on the SHAPE, whose deepest row is the one the expansion mirror gates. + // The table wrapper appears a commit earlier, carrying the root alone. await waitFor(() => - expect( - container.querySelector('[data-testid="object-tree"]') ?? - screen.queryByText('No records'), - ).not.toBeNull(), + expect(drawnRows(container), outcome.because).toEqual(outcome.draws), ); - return container.querySelector('[data-testid="object-tree"]') - ? container.querySelectorAll('tbody tr').length - : 'empty-state'; + return readSettled(container); } +/** Both positive arms draw the same seeded-open hierarchy. */ +const OPEN_FOREST: readonly DrawnRow[] = [ + { label: 'Root', depth: 0 }, + { label: 'Child', depth: 1 }, +]; + afterEach(() => { cleanup(); vi.clearAllMocks(); @@ -121,11 +262,24 @@ afterEach(() => { describe('ObjectTree — the find() envelope it reads (objectui#6839)', () => { it("still reads the contract's `data` member", async () => { - expect(await settledOn(asData), 'the declared rows member must still draw').toBe(2); + const because = 'the declared rows member must still draw the whole forest'; + const settled = await settledOn(asData, { draws: OPEN_FOREST, because }); + expect(settled.rows, because).toEqual(OPEN_FOREST); + // The CONDITION the wait above is keyed to, asserted rather than assumed: + // the child is on screen because the root was seeded OPEN, not because the + // tree flattens its forest regardless of expansion. + expect(settled.rootToggle, 'the root must have settled open').toBe('Collapse'); + expect(settled.tableDrawn, 'the tree-grid must be the node that drew').toBe(true); + expect(settled.emptyPanelDrawn, 'and not the empty panel').toBe(false); }); it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { - expect(await settledOn(asBareArray), 'the bare-array arm must still draw').toBe(2); + const because = 'the bare-array arm must still draw the whole forest'; + const settled = await settledOn(asBareArray, { draws: OPEN_FOREST, because }); + expect(settled.rows, because).toEqual(OPEN_FOREST); + expect(settled.rootToggle, 'the root must have settled open').toBe('Collapse'); + expect(settled.tableDrawn, 'the tree-grid must be the node that drew').toBe(true); + expect(settled.emptyPanelDrawn, 'and not the empty panel').toBe(false); }); it('does NOT read `records` — not a QueryResult member', async () => { @@ -133,9 +287,18 @@ describe('ObjectTree — the find() envelope it reads (objectui#6839)', () => { // declare, and did so AHEAD of `data`. The tree now settles on its own // "No records" panel — a DIFFERENT node from the table, which is what makes // this a reading of the refusal rather than of a mount that never happened. + const settled = await settledOn(asRecords, REFUSES); expect( - await settledOn(asRecords), + settled.emptyPanelDrawn, + 'the refusal must be OBSERVED: the tree drew its "No records" panel', + ).toBe(true); + expect( + settled.rows, 'a `records` envelope must reach the forest as no rows at all, not as the rows it names', - ).toBe('empty-state'); + ).toEqual([]); + expect( + settled.tableDrawn, + 'a refused envelope must not mount the tree-grid at all', + ).toBe(false); }); }); From 17f9ba26e39c051a8f2b6dd1b3bf1a4f45dbaff5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 20:52:36 +0000 Subject: [PATCH 2/2] chore(changeset): declare the plugin-tree envelope-wait test change as releasing nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Empty frontmatter — `node scripts/check-changeset-presence.mjs` names this the explicit exemption for a test-only change under a released package's tree. Part of objectui#6839 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .changeset/tree-contract-envelope-waits-6839.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/tree-contract-envelope-waits-6839.md diff --git a/.changeset/tree-contract-envelope-waits-6839.md b/.changeset/tree-contract-envelope-waits-6839.md new file mode 100644 index 0000000000..a3f4ed734e --- /dev/null +++ b/.changeset/tree-contract-envelope-waits-6839.md @@ -0,0 +1,6 @@ +--- +--- + +Test-only change in `@object-ui/plugin-tree`: the `contractEnvelope-6839` pin now +splits its waits by expected outcome instead of reading the row count the instant +the tree-grid wrapper mounts. No published behaviour changes.