Repository navigation
test(Selector, MultiSelector): load the lazy BottomSheet before the sheet tests - #9082
Conversation
…heet tests The bottom-sheet presentation renders BottomSheet through React.lazy; its first import transforms the BottomSheet module tree, which on a loaded runner takes longer than findByRole's 1 s wait, so the first sheet test in each file timed out before the dialog existed. Import the module once in beforeAll so lazy() resolves from the module cache.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsMultiSelector (@astryxdesign/core) · View in Storybook
Component Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good to merge: preloading ../BottomSheet/BottomSheet in a file-level beforeAll takes the lazy sheet's first module load out of the first sheet test's 1 s findByRole window without changing any assertion, the same pattern PopoverPresentation.test.tsx already uses. Both test files pass on this head, and the first sheet test in each now runs roughly a second faster than on main. One optional tidy-up: the existing // Mock showPopover and hidePopover… comment now sits above the new block instead of the beforeEach it describes (see the inline note).
[Automated review]
| } | ||
|
|
||
| // Mock showPopover and hidePopover methods since they're not implemented in jsdom | ||
| // The bottom-sheet presentation renders BottomSheet through React.lazy. Its |
There was a problem hiding this comment.
Nit (non-blocking): this block landed between the existing // Mock showPopover and hidePopover methods… comment on line 75 and the beforeEach it describes, so that comment now reads as if it labels the BottomSheet preload. Moving the new comment and beforeAll above line 75 keeps each comment with its code. Same in Selector.test.tsx at lines 71-72.
Maintainer impact
"uses a bottom sheet and stays open while toggling options" (MultiSelector) and "uses a bottom sheet and closes after a selection when requested" (Selector) fail at random on loaded CI runners with
Unable to find role="dialog" and name "Fruit". They block unrelated pull requests: #9018 failedtest-ui (4/4)on two attempts, and #9017 and #9028 hit the same miss. On a busy local machine (load average ~70), the MultiSelector file failed 3 of 3 runs on main.Intended invariant
The sheet tests keep asserting the same behavior: the bottom-sheet presentation opens a dialog named by the label, and toggling or selecting keeps or closes it as before. Only the time to first load the BottomSheet module leaves the measured window.
Change
Both test files import
../BottomSheet/BottomSheetonce in a file-levelbeforeAll.SelectorBottomSheetrenders BottomSheet throughReact.lazy. The firstimport()in a test file transforms the whole BottomSheet module tree, which under load takes longer thanfindByRole's 1 s default, so the first sheet test in each file timed out before the dialog existed. With the module already loaded,lazy()resolves from the module cache and each sheet test waits only for the open itself. This removes the cause instead of raising the query timeout. No product code changes.The original sheet tests come from #5395 (rubyycheung).
Evidence
MultiSelector.test.tsxon main, 3 runs under load average ~70: 3/3 failed (1 test each). CI: fix(Selector, MultiSelector): a light dismiss onto another field leaves focus there #9018test-ui (4/4)failed twice with the same error.MultiSelector.test.tsx+Selector.test.tsx, 3 runs under load average ~65: 420/420 passed each time.Scope
Testing
pnpm vitest run packages/core/src/MultiSelector/MultiSelector.test.tsx packages/core/src/Selector/Selector.test.tsx(3× under load).