test(plugin-tree): split the contractEnvelope-6839 waits by expected outcome

.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.

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<Set<string>>(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<number | 'empty-state'> {
+async function settledOn(envelope: Envelope, outcome: Outcome): Promise<Settled> {
   const find = vi.fn(async () => envelope(ROWS));
   const ds: any = {
     find,
@@ -103,39 +223,82 @@ async function settledOn(envelope: Envelope): Promise<number | 'empty-state'> {
   // 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();
 });
 
 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 () => {
     // Before the fix these two nodes drew off a key `QueryResult` does not
     // 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);
   });
 });