Uh oh!
There was an error while loading. Please reload this page.
test(plugin-gantt): read the chart through an async query, not a race - #6966
Merged
Conversation
`ObjectGantt.referenceArms-6837.test.tsx` mounted through its `mount()` helper
— whose gate settles a MOCK CALL, `find('task', { $expand })` was ISSUED — and
then read the DOM synchronously with `getByTestId('gantt-view')`. Those are two
different facts, one promise resolution apart: `ObjectGantt` flips `loading`
false in the `finally` of `reload()`, i.e. after that find RESOLVES, so on
return from `mount()` the component can still be painting `Loading Gantt
chart...`. The synchronous read raced it and lost twice in fifteen minutes in
the merge queue, ejecting two pull requests that do not touch `plugin-gantt`.
The assertion is unchanged and `mount()` keeps its find-call gate: the refusal
probe above needs proof of the SCHEMA-dependent commit, which the chart's
presence alone does not carry, since `loading` flips false after the FIRST find
resolves and before `objectSchema` lands. The block now waits for both facts
rather than trading one for the other.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yBContributor
✅ 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
|
This was referenced Aug 31, 2026
os-sam
marked this pull request as ready for review
August 31, 2026 04:16
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6959
packages/plugin-gantt/src/ObjectGantt.referenceArms-6837.test.tsx:257read the chart with a synchronousgetByTestIdand lost the race twice in fifteen minutes in the merge queue, ejecting two PRs that do not touchplugin-gantt. This converts that read — and the sibling read on the next line — to the house async form. The assertion is unchanged. Nothing is skipped, quarantined, or timeout-inflated, andObjectGanttitself is untouched.One correction to the card's mechanism, stated rather than applied silently
The card attributes the fallback DOM to a
React.lazyboundary. It is not one.ObjectGantt.tsx:65importsGanttViewstatically, and there is noReact.lazyanywhere inpackages/plugin-gantt.Loading Gantt chart...is theif (loading)early return atObjectGantt.tsx:1580-1587, driven byuseState(true)plus the async data/schema fetch.Everything else in the card holds, and the remedy is identical — this is a synchronous read of an asynchronously-appearing surface either way. The correction matters only for where the window is, and that is what made the probe below constructible.
The actual race
mount()gates on a mock call —find('task', { $expand })was ISSUED.ObjectGanttflipsloadingfalse in thefinallyofreload(), i.e. after that find resolves. Those are two facts one promise-resolution apart, so on return frommount()the component can still be painting its loading branch.mount()'s find-call gate is kept, deliberately. The refusal probe above this block needs proof of the schema-dependent commit, and the chart's presence alone does not carry it:loadingflips false after the first find resolves, beforeobjectSchemalands. Swapping one gate for the other would have weakened the refusal pin. The block now waits for both facts.Measurement — the timing fact mutated, never the assertion
A throwaway probe copied the file's mocks, schema and
mount()verbatim and changed exactly one thing:findresolves 500 ms late, as on a loaded queue runner. Both legs ran; the probe was deleted and is not in this PR.DOM at read time: "Loading Gantt chart...", and the sync read threwUnable to find an element by: [data-testid="gantt-view"]. That reproduces the queue failure in the card.await findByTestIdresolved through that fallback;data-count=2.At 25 ms the probe did not reproduce: the window closed before
waitFor's 50 ms poll observed the call. That negative reading is the point — locally the window is sub-50 ms, which is exactly why this passes on a quiet runner and fails on a loaded one.The census — is
:257the only one?No. Bound, stated:
git ls-files -- '*.test.tsx' '*.test.ts' '*.spec.tsx' '*.spec.ts'= 2282 files. Hot control: every tier below reports:257itself. Tier 1 initially did not — it required a literalrender(in the block and:257mounts through a helper — the control caught that and the instrument was fixed rather than the corpus declared clean.Tier 3 is the shape: the awaited settle never touches the DOM.
In
packages/plugin-gantt(in scope) tier 3 returns 3, and hand triage leaves exactly one real defect —:257. The other two are false positives:ObjectGantt.quickfilter.test.tsx:141is gated at:135through the file-localgv()helper, andGanttView.autoscheduledlg.test.tsx:63has a synchronous mount with no async settle to race. The 10 tier-2 plugin-gantt hits triage the same way — every one has a real DOM gate, several hidden behindgv()/renderSettled().173 tier-3 candidates sit outside
plugin-ganttand are deliberately NOT touched here — several of those packages have live PRs. Assume roughly the 2-in-3 false-positive rate measured inplugin-gantt; the concentrations areapp-shell58,fields28,apps/console25,components16,plugin-form11,plugin-grid9. These are reported to the PM for separate dispatch, not filed as findings from this seat.Changeset
Owed, and added with empty frontmatter.
scripts/check-changeset-presence.mjsguards all of each package'ssrc/with "No carve-out for test files undersrc/" stated explicitly in its header, and@object-ui/plugin-ganttis in thefixedgroup. Verified in both directions: the gate exits 1 before the changeset and 0 after, quotingThe
skip-changesetlabel is not used: in this repo it is an inert historical label that exempts nothing, and the empty-frontmatter changeset is the declaration mechanism.Verification — union run on the final commit,
6f5c01fb5Joined with
&&so the verdict covers every part; run through the shared verify lock.pnpm exec vitest run packages/plugin-gantt/— Test Files 53 passed (53), Tests 434 passed (434)pnpm --filter @object-ui/plugin-gantt run type-check— clean. Measured, not assumed:tsc --noEmit --listFilescontains 0 hits for the edited file (the package tsconfig excludes tests); thetsconfig.test.jsonleg contains 1. Coverage comes from the second leg.check:control-bytes,check:vi-mock-specifiers,check:vi-mock-inherit,check:shell-escape-residue— all✅ OKcheck-changeset-presence/-no-major/-fixed/-overwrite— all✅The vitest invocation matters:
pnpm --filter PKG exec vitestis refused by this repo's guard, which reports that it silently runsapps/console's 22 files and printsTest Files 22 passedwhile running none of this package's. The guard fired on my first attempt; the runs above are the root-level form it prescribes.Repo-wide
pnpm lintwas narrowed, and the narrowing is measured rather than asserted:files: ['**/*.{ts,tsx}']plus the config'signores.--format json— 1 file linted, 0 errors, 6 warnings.languageOptionsdeclares onlyecmaVersionandglobals; there is noproject/projectService, so type-aware linting is off and no rule's verdict on any file can depend on another file's contents.The 6 warnings are pre-existing: linting the base blob of the same file gives 6 warnings / 0 errors, all
@typescript-eslint/no-explicit-any— identical before and after. CI deliberately sets no--max-warnings.Generated by Claude Code
Generated by Claude Code