From b7ecaf125dbeca369fa10d659c00723fc06080f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 07:20:14 +0000 Subject: [PATCH] test(plugin-kanban): split the contractEnvelope-6839 waits by expected outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pin waited on the `'Negotiation'` column header and then read the cards synchronously. Measured here, that header is not a first-paint signal: the board reaches `KanbanImpl` through `React.lazy` behind a `Suspense`, so the header appears only when that chunk resolves — after `render`, after `find` is called, and after `find` settles. `KanbanImpl` mirrors its `columns` prop into `boardColumns` state, and both the header text and the cards draw from that mirror, so the header lands in whatever state the mirror was seeded with at its own mount. Chunk load and data commit are two independent races and nothing ordered them; when the chunk won, the reveal drew an empty list and the read saw 0. That is the `expected +0 to be 2` failure on shard 2/4. The four cases need opposite waits, so they no longer share one. The three positive arms wait FOR the rows, which is immune to which race won. The `records` refusal arm has no arrival to wait for, so it takes a settled read: `find` has answered, the board has drawn the card list itself (not merely the header), and React's queued work is flushed. Waiting on the list is what separates "the list is empty" from "the list is absent" — `cards()` answered `[]` for both. No timeout was raised, no case skipped, and `cardsThrough` is still called once per case and never inside a `waitFor` predicate (objectui#7802). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- ...8532-kanban-contract-envelope-rows-wait.md | 10 ++ ...bjectKanban.contractEnvelope-6839.test.tsx | 118 +++++++++++++++--- 2 files changed, 110 insertions(+), 18 deletions(-) create mode 100644 .changeset/8532-kanban-contract-envelope-rows-wait.md diff --git a/.changeset/8532-kanban-contract-envelope-rows-wait.md b/.changeset/8532-kanban-contract-envelope-rows-wait.md new file mode 100644 index 0000000000..edb948ffbf --- /dev/null +++ b/.changeset/8532-kanban-contract-envelope-rows-wait.md @@ -0,0 +1,10 @@ +--- +--- + +Test-only (objectui#8532). `ObjectKanban.contractEnvelope-6839` waited on the +`'Negotiation'` column header — a signal the `React.lazy` chunk reveals +independently of the data commit — and then read the cards synchronously, so +the pin went red on `main` on PRs that cannot reach `plugin-kanban`. The three +positive arms now wait FOR the rows; the `records` refusal arm, which has no +arrival to wait for, takes a settled read against the card list itself rather +than the header. No published source changed. diff --git a/packages/plugin-kanban/src/ObjectKanban.contractEnvelope-6839.test.tsx b/packages/plugin-kanban/src/ObjectKanban.contractEnvelope-6839.test.tsx index 9dcaf19459..773518379e 100644 --- a/packages/plugin-kanban/src/ObjectKanban.contractEnvelope-6839.test.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.contractEnvelope-6839.test.tsx @@ -33,10 +33,50 @@ * `find`). CONTROL, so the zero is a reading: the same sweep finds a live * `find()` double emitting `{ records: [...] }` at `plugin-list`'s * ObjectGallery, a consumer with its own unwrap ladder. + * + * ## ⭐ Why the two outcomes wait DIFFERENTLY (objectui#8532) + * + * This file used to hand all four cases one wait, tuned for the refusal — and + * it went red on `main` on PRs that cannot reach `plugin-kanban` at all. + * + * The wait was `waitFor(() => screen.queryByText('Negotiation'))`, and its own + * docblock called that a MOUNT signal rather than a rows signal. MEASURED here, + * the header is weaker still: it is not a first-paint signal either. The board + * reaches `KanbanImpl` through `React.lazy(() => import('./KanbanImpl'))` behind + * a `Suspense` (`src/index.tsx`), so at `render`, at `find` being CALLED, and at + * `find` being SETTLED the header is still absent — it appears only when that + * chunk resolves. `KanbanImpl` then mirrors its `columns` prop into + * `boardColumns` state and re-syncs it through a `useEffect`, and BOTH the + * header text and the cards are drawn from that mirror. So the header lands in + * whatever state the mirror was seeded with at ITS mount. + * + * The header and the rows are therefore two INDEPENDENT races — chunk load + * versus data commit — and nothing in the helper orders them. When the data + * commit wins, the reveal carries the rows and the read sees 2 (every local + * run, cold chunk). When the chunk wins, the reveal draws the column with an + * empty list and the read sees 0 — the CI failure, `expected +0 to be 2`. + * + * The four cases need OPPOSITE waits, so they no longer share one: + * + * - the three POSITIVE arms wait FOR the rows. A row count is a signal you + * can wait on, and waiting on it is what makes them immune to which race + * won — not a wider window on the same race, which is what a raised + * timeout would have bought. + * - the REFUSAL arm cannot wait for an absence, so it takes a SETTLED read: + * `find` has answered, the board has drawn the very list `cards()` reads, + * and everything React still had queued is flushed. Waiting on the LIST + * rather than on the header is the part that matters — `cards()` answers + * `[]` both when the list is ABSENT and when it is EMPTY, so under the old + * header wait a board that had not drawn yet was indistinguishable from a + * board that refused. + * + * ⛔ 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 { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { render, screen, waitFor, cleanup, within } from '@testing-library/react'; +import { render, screen, waitFor, cleanup, within, act } from '@testing-library/react'; import React from 'react'; import type { DataSource, ObjectKanbanSchema } from '@object-ui/types'; import { ObjectKanban } from './ObjectKanban'; @@ -74,29 +114,58 @@ const asBareArray: Envelope = (rows) => rows; const asValue: Envelope = (rows) => ({ value: rows, total: rows.length }); const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); +/** + * The column's card list — the ONE node `cards()` reads. + * + * The board draws it on EVERY arm, refused or not (`KanbanColumnView` renders + * it unconditionally; a zero-card column fills it with a dashed placeholder + * that is not a `listitem`). So its presence is the honest "this column has + * been drawn" signal, and it is what separates an empty list from a missing one. + */ +function cardList(): HTMLElement | null { + return screen.queryByRole('list', { name: 'Negotiation cards' }); +} + /** Cards the board actually painted, by their `aria-label`. */ function cards(): string[] { - const list = screen.queryByRole('list', { name: 'Negotiation cards' }); + const list = cardList(); if (!list) return []; return within(list) .queryAllByRole('listitem') .map((el) => el.getAttribute('aria-label') ?? ''); } +/** + * What a case expects the board to settle on — which is also what decides HOW + * it waits. See the `objectui#8532` section of this file's header. + */ +type Outcome = + /** Wait FOR the rows. `because` is carried into the timeout message. */ + | { readonly draws: number; readonly because: string } + /** No absence to wait for: settle, then read. */ + | { readonly refuses: true }; + +const REFUSES: Outcome = { refuses: true }; + /** * Mount the board over a `find()` answering `envelope`, and hand back the cards - * it drew. + * it drew once `outcome` says the board has settled. * * ⛔ Call this ONCE per case and NEVER from 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: `cards()` and `cardList()` are pure `screen` reads. They + * mount nothing, so re-running them on a DOM mutation is free — which is + * exactly the property `cardsThrough` itself does not have. + * * ⚠️ Mounted with NO `data` prop: `ObjectKanban` skips its own fetch when * external data is supplied, and a board handed its rows directly would answer * every case identically — measuring nothing. */ -async function cardsThrough(envelope: Envelope): Promise { +async function cardsThrough(envelope: Envelope, outcome: Outcome): Promise { const find = vi.fn(async () => envelope(ROWS)); const ds = { getObjectSchema: vi.fn(async () => DEF), @@ -112,10 +181,22 @@ async function cardsThrough(envelope: Envelope): Promise { // touches no DOM. Without it, "no cards" is satisfied by the mount's initial // empty state, which every arm renders identically. await find.mock.results[0].value; - // The column header lands on every arm, refused or not, so it is a mount - // signal rather than a rows signal — which is exactly what makes it the - // right thing to wait on before reading the cards. - await waitFor(() => expect(screen.queryByText('Negotiation')).toBeTruthy()); + + if ('refuses' in outcome) { + // A refusal has no arrival to wait for, so this is a SETTLED read, built + // from the three things that CAN be observed: `find` has answered (above), + // the board has drawn the list itself — not merely the header, which the + // lazy chunk can reveal ahead of the data — and React has nothing left + // queued. `act` here is a flush of the pending work, not a delay: it is + // the opposite of widening a timeout. + await waitFor(() => + expect(cardList(), 'the board must have drawn the column list before it is read').not.toBeNull(), + ); + await act(async () => {}); + return cards(); + } + + await waitFor(() => expect(cards(), outcome.because).toHaveLength(outcome.draws)); return cards(); } @@ -130,28 +211,29 @@ afterEach(() => { describe('ObjectKanban — the find() envelope it reads (objectui#6839)', () => { it("still reads the contract's `data` member", async () => { - const drawn = await cardsThrough(asData); - expect(drawn.length, 'the declared rows member must still draw both cards').toBe(2); + const because = 'the declared rows member must still draw both cards'; + const drawn = await cardsThrough(asData, { draws: 2, because }); + expect(drawn.length, because).toBe(2); }); it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { - const drawn = await cardsThrough(asBareArray); - expect(drawn.length, 'the bare-array arm must still draw both cards').toBe(2); + const because = 'the bare-array arm must still draw both cards'; + const drawn = await cardsThrough(asBareArray, { draws: 2, because }); + expect(drawn.length, because).toBe(2); }); it('still reads `value` — LIVE at this seam, three doubles in this package emit it', async () => { - const drawn = await cardsThrough(asValue); - expect( - drawn.length, + const because = 'objectui#6840 refused to transfer its `ObjectView` zero here; deleting this arm would ' - + 'break three doubles in this package', - ).toBe(2); + + 'break three doubles in this package'; + const drawn = await cardsThrough(asValue, { draws: 2, because }); + expect(drawn.length, because).toBe(2); }); it('does NOT read `records` — not a QueryResult member', async () => { // Before the fix these two cards drew off a key `QueryResult` does not // declare, and did so AHEAD of `data`. - const drawn = await cardsThrough(asRecords); + const drawn = await cardsThrough(asRecords, REFUSES); expect( drawn, 'a `records` envelope must reach the board as zero cards, not as the rows it names',