test(plugin-detail): settle the fetched value before two RelatedList tests read it - #7757
Conversation
…tests read it `RelatedList.serverpagination.test.tsx` gated its page-3 read on the page label alone and then read `h.schema.data`. The label is pagination state — `currentPage` flips on the click and `totalPages` came from page one's `total` — so "Page 3 of 3" commits before the third window reaches `relatedData`, and the assertion samples the previous page. That is why the CI value was 5, the page size. Measured in this run, on 81a2eb1: deferring only the `$skip >= 10` window by 50ms inside the fetch effect reproduces the CI failure byte-for-byte — `expected [ { id: 'c5', name: 'Row 5' }, …(4) ] to have a length of 2 but got 5` at RelatedList.serverpagination.test.tsx:118:27. The repair keeps the label gate (it proves the view committed) and adds a gate on the value the next line reads. ⛔ Not a `toHaveBeenCalledWith` gate on `find`: a mock call is issued one resolution before its rows reach state (objectui#6959), so that would flake less often and fail identically. A census over `packages/plugin-detail/` with the same instrument found a second site of the same class: `RelatedList.wildcardInvalidation.test.tsx` gates on `find` having been CALLED once, then closes by reading the row that call produces. Same one-line repair — settle the value before the foreign events go out, so "still line-1" is a statement about the guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
PM ruling on both open questions — A and A. One of them corrects my own dispatch brief.
Q1 — the changeset: ⛔ my brief was wrong, the repo governs, and the seat was right to say soMy brief said: "A changeset is required only if a released package's source changed — it should not have; if you need one, that is the Clause-② void condition, so stop and report." That contradicts this repo's binding rule.
⇒ Ruling: A. The void condition my brief actually defines is a change under ⭐ Option B would have left a red mechanical gate on a green PR in order to obey a sentence I wrote from memory instead of from Q2 — the page-2 gate: A, and the measurement is what decides itThe card asked for a judgment; the seat produced a reading rather than a shape argument, and it changes the answer. Under probes A and B — both of which delay the page-2 refetch by 50ms — the assertion that follows that gate, ⇒ It is the same shape as the repaired defect but not the same defect. And its Ruling: A — leave it, and keep it named in the PR body as a latent trap for whoever later adds a data read after it. The census bound, stated as a boundThe package-wide census is NOT MEASURED, not a zero —
Landing once CI is terminal — 22 success / 3 skipped / 0 failed, 7 still running as of this comment. Generated by Claude Code |
2 participants
Fixes #7579
Two RelatedList tests read a value out of a chain their gate never settles. Both
now gate on the value itself. No runtime source is touched.
The defect, and the reproduction
RelatedList.serverpagination.test.tsx:117-118gated the page-3 read on the pagelabel and then read the refetched rows:
Read from
RelatedList.tsxon this base:totalPagesisMath.ceil(total / pageSize)whenever the server reported a
total, andtotalarrived with page one;currentPageflips synchronously on the click. So "Page 3 of 3" commits on the click's own render,
before the third window reaches
relatedData, and the assertion samples the previouspage — which is why CI's received value was 5, the page size.
Instrument. A timing FACT mutated inside the component and nothing else: in the fetch
effect, the resolution of
dataSource.findis deferred by 50ms. Three variants were used,all of which delay only when rows land, never what lands:
$skip— the page-flip refetch chain$skip >= 10request — the third window aloneMeasured on the base
81a2eb1fb, probe C:That is the CI assertion string, received value included, reproduced deterministically.
Probe A produces the same failure at the same line with
c0in the received value, becauseit delays the page-2 refetch as well and state is then one window further back.
Each probe leg proved itself on disk before running (anchor 1 to 0, marker 0 to 1, a
git diff --numstatnaming the file) and restored viagit checkout HEAD -- ABSOLUTE_PATHwith
git hash-objectequal to the HEAD blob and an emptygit diff HEAD --stat, under atrap ... EXIT INT TERM.The census this card asked for, and what it found
#7075's census deferred
getObjectSchema, which probes the schema chain; this site rides therefetch chain, so that probe could not have reached it. Re-run here over the whole package,
pnpm exec vitest run packages/plugin-detail/:Test Files 132 passed (132),Tests 1199 passed (1199)1 failed / 132,1 failed / 1199—RelatedList.serverpagination.test.tsx:1182 failed / 132,2 failed / 1199— the above, plusRelatedList.wildcardInvalidation.test.tsx:133Lit control: the site this card names goes red under all three probes, so a zero
elsewhere is a reading and not a dark instrument.
Dark control, which is what makes the probe discriminating rather than "a delay breaks
tests":
RelatedList.serverpagination.test.tsx:244has the same text shape — click,await screen.findByText('Page 2 of 2'), thenexpect(h.schema.data).toHaveLength(2)— andstays GREEN under every probe. Its label is derived from
hasMore, whichsetStates in thesame batch as the rows, so that label gate genuinely settles the value. Shape alone does not
predict the defect; the chain behind the label does.
The second site, and why it is the same defect
RelatedList.wildcardInvalidation.test.tsxcloses its guard test by reading the row thefetch produced, behind a gate that settles the CALL:
Same one-line repair, so it is repaired here: settle the value before the foreign events
go out, which also makes the closing read non-vacuous — "still line-1" becomes a statement
about the guard rather than about rows that had not landed yet.
What the repair is, and what it deliberately is not
Both sites keep their existing gate and add a gate on the value the next read rides. Neither
is a
toHaveBeenCalledWithgate onfind: a mock call is issued one resolution before itsrows reach state (#6959), so a call gate would flake less often and fail identically. No
timeout was widened, nothing was skipped, no expected length was relaxed.
The page-2 gate in the same test was examined and deliberately left alone. It is the same
shape, but measurement says it is not the same defect: under probes A and B — both of which
delay the page-2 refetch — the assertion that follows it,
expect(prevButton().disabled).toBe(false),stayed green, because
disablediscurrentPage === 0, pure pagination state the click alreadycommitted. Nothing there reads the fetch chain. Its
toHaveBeenCalledWithis also doing adifferent job from a gate: it pins the WIRE,
{ $filter, $top: 5, $skip: 5 }, which is thecontract this test exists to hold. Converting it to a value gate would delete that coverage.
It stays as a latent trap for whoever later adds a data read after it, and it is named here so
that is a decision rather than an accident.
The repaired gates can still fail
A gate that can no longer fail is worse than the flake, so the behaviour — not the timing —
was broken under the fix:
$skip >= 10): red at the new gate,RelatedList.serverpagination.test.tsx:127:47in themutated file, and the test's own duration goes 67ms green to 1062ms red — the
waitForexhausting its window and rethrowing, not a fast assertion.
wildcardInvalidation's fixture returns no rows): red at thenew gate,
:130:57.Both mutations were proved on disk and restored with a HEAD-blob hash comparison under a trap.
Verification
Repo root, exit codes captured before any pipe, all on
9c142c03a:pnpm exec vitest run packages/plugin-detail/132 passed (132),1199 passed (1199), exit 012 passed (12)in every mode, exit 0pnpm --filter @object-ui/plugin-detail type-checkpnpm --filter @object-ui/plugin-detail lintcheck:control-bytes/check:vi-mock-specifiers/check:vi-mock-inheritnode scripts/check-changeset-presence.mjspnpm --filter @object-ui/plugin-detail^... buildran first, sotsconfig.test.jsonresolved@object-ui/*through freshly built.d.ts. That project's--listFilesnames both editedfiles once each, so "type-check is clean" is a statement about this diff and not a project
that excludes tests.
Declared narrowing: lint here is the affected package's own
eslint ., not the repo-wideturbo run lint.eslint.config.jsconfigures no type-aware linting — noprojectand noprojectService— so no verdict on an untouched file can move because of a change inside twotest files in one package. The repo-wide farm is CI's run.
Changeset
.changeset/7579-relatedlist-value-gates.mdcarries an empty frontmatter — this repo'sdeclared "releases nothing" form.
check-changeset-presence.mjsrequires a changeset for anytouch under a released package's
src/, andsrc/__tests__/is inside that. It is adeclaration, not a version bump: no published behaviour changes and no runtime source is in
this diff.
Follow-up filed, not left in prose
A fourth probe, deferring the lookup-label batch by the same 50ms, was designed and run. Its
package-wide result is NOT MEASURED, not a zero: the vitest worker dies of V8 heap
exhaustion. Running it file-by-file isolated that to one file,
RelatedList.lookupLabelResolution.test.tsx, which renders into the DOM inside awaitForpredicate. That is a different failure mode with a different repair, so it is not touched here
and is filed with its measurement as #7756 — out of scope for this PR, and not left in prose.
Generated by Claude Code