Migrate the whole test suite to Vitest browser mode and delete Cypress tests - #2663
Merged
Conversation
Contributor
|
Contributor
Visual regression report
Diff images (33)alert.png — baseline no longer producedavatar.png — baseline no longer producedbadge.png — baseline no longer producedbillboard.png — baseline no longer producedbreadcrumb.png — baseline no longer producedbutton-and-derivatives.png — baseline no longer producedbyline.png — baseline no longer producedcalendar.png — baseline no longer producedcheckbox.png — baseline no longer producedcheckboxgroup.png — baseline no longer producedcolorpicker.png — baseline no longer producedcontextview.png — baseline no longer producedcustom-and-lucide-icons.png — baseline no longer produceddateinput-dateinput2.png — baseline no longer produceddatetimeinput.png — baseline no longer produceddiff-demo.png — 6324 pixels differdrilldown.png — baseline no longer producedfiledrop.png — baseline no longer producedform-errors.png — baseline no longer producedheading.png — baseline no longer producedimg.png — baseline no longer producedlink.png — baseline no longer producedmenu.png — baseline no longer producedmetric-pill-tag-timeselect-text.png — baseline no longer producedoptions.png — baseline no longer producedpagination.png — baseline no longer producedprogressbar.png — baseline no longer producedselect-simpleselect.png — baseline no longer producedtable.png — baseline no longer producedtabs.png — baseline no longer producedtooltip.png — baseline no longer producedtreebrowser.png — baseline no longer producedview.png — baseline no longer producedBaselines come from the |
matyasf
commented
Aug 4, 2026
| @@ -1,32 +1,8 @@ | |||
| { | |||
| "compilerOptions": { | |||
Collaborator
Author
There was a problem hiding this comment.
turns out that tsconfig.json can be very similar to tsconfig.build.json, this makes the tests not emit TS errors (because the old one was using an old value for moduleResolution)
matyasf
force-pushed
the
vitest_browser_convert
branch
3 times, most recently
from
August 4, 2026 19:42
0a3608d to
9803b43
Compare
The suite was split across two Vitest projects: a `web` project running most tests in jsdom, and a `browser` project running a handful of packages in a real Chromium via Playwright. jsdom's approximations meant tests could pass while the component was broken in a browser (no layout, no real focus, no computed styles), so the `browser` project now owns every `packages/**/__tests__` file and the jsdom project is gone. - collapse the `web` and `browser` projects into one `browser` project and drop the per-package include/exclude lists that kept them from double-running - delete `vitest.setup.ts` — its jsdom shims (the `ResizeObserver` mock and the hardcoded `16px` root font size) are unnecessary in a real browser - `vitest.setup.browser.ts` restores mocks after each test; jsdom's setup did this and browser tests need it too, otherwise a `console.warn` spy from one test observes the next one's output - stop excluding `src/**/__tests__/**` from the packages' `tsconfig.build.json` and have the root `tsconfig.json` extend `tsconfig.build.json`, so test files are type-checked with the same options as the source - collapse the two Vitest CI jobs into one Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
matyasf
force-pushed
the
vitest_browser_convert
branch
from
August 4, 2026 20:48
9803b43 to
626e4c9
Compare
matyasf
commented
Aug 4, 2026
| // built `es/` output. | ||
| // This trades DX for speed — after editing the *source* of one | ||
| // of these, run that package's `build` before browser tests will see the change. | ||
| const PREBUNDLED_PACKAGES = ['@instructure/ui-icons', '@instructure/ui-themes'] |
Collaborator
Author
There was a problem hiding this comment.
I am unsure, that this is a good change. The tests run in:
- ~45 secs is nothing is pre-bundled
- ~25 secs if only
ui-iconsis pre-bundled - ~15 secs if
ui-iconsandui-themesare pre-bundled - (~11 secs if everything is pre-bundled, but in this case one needs to rebuild the component(s) to see changes)
matyasf
force-pushed
the
vitest_browser_convert
branch
2 times, most recently
from
August 5, 2026 09:55
46f0b5c to
656ec13
Compare
matyasf
force-pushed
the
vitest_browser_convert
branch
from
August 5, 2026 10:06
656ec13 to
efed3a7
Compare
matyasf
force-pushed
the
vitest_browser_convert
branch
from
August 5, 2026 10:17
efed3a7 to
f1201fd
Compare
matyasf
force-pushed
the
vitest_browser_convert
branch
from
August 5, 2026 10:27
f1201fd to
d5ef351
Compare
matyasf
force-pushed
the
vitest_browser_convert
branch
2 times, most recently
from
August 5, 2026 10:44
0c628d9 to
1279d8d
Compare
Mechanical, codemod-driven conversion of all 181 test files off React Testing
Library and onto the vitest-browser API. No test's intent changes here; the
follow-up commits deal with the cases that need real thought.
The substitutions:
- `render` now comes from `vitest-browser-react` and returns a promise, so every
call becomes `await render(...)`.
- `screen.getBy*` -> `page.getBy*(...).element()`, `getAllBy*` -> `.elements()`,
`queryBy*` -> `.query()`; vitest-browser locators are lazy, so the `.element()`
call is what actually resolves the node.
- `userEvent` now comes from `vitest/browser`
- `waitFor` -> `vi.waitFor`
- `fireEvent` now comes from `@testing-library/dom` directly
- `import '@testing-library/jest-dom'` and
`import { describe, it, expect, ... } from 'vitest'` becomes an explicit
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A real browser does layout, real hit-testing, real focus and real event ordering,
and a fair number of tests were relying on jsdom not doing any of that.
The fixes:
- `userEvent.click(..., { force: true })` in ~36 places. Our inputs are visually
hidden and covered by a styled facade, so a real click lands on the facade and
Playwright refuses the click as intercepted.
- swap the remaining `fireEvent.click` for `userEvent.click` where the test is
about a user clicking. `fireEvent` dispatches one synthetic event; only a real
click reproduces the browser's own default handling.
- fake timers replaced by `vi.waitFor` and `expect.element(...)`. jsdom needed
`act(() => vi.runAllTimers())` to flush transitions synchronously; in a browser
the transitions are real, so the tests wait for the end state instead.
- a couple of keyboard assertions were simply wrong. A real checkbox toggles on
Space, not Enter — jsdom fired the `change` handler for either.
- Tray/Popover/Dialog tests now `cleanup()` and let the teardown drain in
`afterEach`, because `FocusRegion` unregisters its document listeners
asynchronously and a leaked `keydown` listener from one test was swallowing the
Tab presses of the next test.
- helpers that wrap `render` are awaited
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 56 tests under `cypress/component/` move into the matching `__tests__`
files, mostly as a nested `describe('Component tests', ...)`.
The translation of Cypress idioms:
- `cy.mount` -> `await render`
- `cy.get(...).should(...)` -> `page.getBy*` / `expect(...)` inside `vi.waitFor`,
since Cypress retried assertions implicitly and Vitest does not
- `cy.realClick` / `cy.realHover` (cypress-real-events) -> `userEvent` from
`vitest/browser`
- `cy.tick` -> `vi.advanceTimersByTime`
Two specs land in new files: `Truncate.cy.tsx` becomes `truncate.test.tsx` and
`i18n.cy.tsx` becomes `getTextDirection.test.tsx`, because neither had a unit
test file to merge into.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every spec is now a vitest-browser test co-located with its component, so the whole Cypress component-testing setup is deleted. The testing docs are updated to match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
matyasf
force-pushed
the
vitest_browser_convert
branch
from
August 5, 2026 12:12
1279d8d to
0843238
Compare
matyasf
changed the title
`DateInput.cy.tsx` was the largest Cypress spec. Merging all of
it into `DateInput.test.tsx` would have made a 1700+ lines file, so it was
split into 2 files.
Splitting means the shared plumbing needs a home, so `dateInputTestHelpers.ts`
holds what both files use. Two other changes for performance were made:
- `clickElement` dispatches mousedown/mouseup/click via `fireEvent` instead of
`userEvent.click`, which is much faster (around 10x).
- `waitFor2ms` wraps `vi.waitFor` with `{ interval: 2 }`. The default 50ms poll
interval is too much when the thing being waited for already happened on the
same tick, which is the case for every one of these DOM assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Now ui-icons and ui-themes are resolved to their built `es/` output. This roughly halves the test suite's runtime. The cost is a DX trade-off: after editing the *source* of either package you have to build it before browser tests see the change. Vite does not notice any changes in the `es/` output, so a rebuild would not triggeer a rerun on the tests. To fix this, the `instui-prebundle-stamp` plugin is added to caclulate packages' modified times into that cache key, so a rebuild changes the plugin name and invalidates the bundle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`View.omitViewProps` built its dev warning from `Component.name`, but the components that call it are wrapped by `@withStyle`, so `name` is empty, so the warnings read `[undefined] prop 'x' is not allowed.` Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
matyasf
force-pushed
the
vitest_browser_convert
branch
from
August 5, 2026 12:58
0843238 to
89d8460
Compare
matyasf
commented
Aug 5, 2026
Comment on lines
+102
to
+106
| const name = Component.displayName | ||
| ? Component.displayName | ||
| : Component.name | ||
| Object.keys(pickProps(props, propsToOmit)).forEach((prop) => { | ||
| error(false, `[${Component.name}] prop '${prop}' is not allowed.`) | ||
| error(false, `[${name}] prop '${prop}' is not allowed.`) |
Collaborator
Author
There was a problem hiding this comment.
This (and the same change for v1) is the only code change
joyenjoyer
requested changes
Aug 6, 2026
joyenjoyer
left a comment
joyenjoyer
left a comment
Contributor
There was a problem hiding this comment.
Please take a look at my comments here: eb53eb3#diff-a6ee97c84dbd70f44ac3ed51e0a2be0bfe0ef24529be5407eb92a3cb7e8cc518
Those are minor stuff but go through them anyways, very nice work!
matyasf
force-pushed
the
vitest_browser_convert
branch
from
August 6, 2026 15:47
16b45ff to
2f2f476
Compare
HerrTopi
approved these changes
Aug 7, 2026
HerrTopi
left a comment
HerrTopi
left a comment
Contributor
There was a problem hiding this comment.
Only question is if you thought about the testing documentation. Does it need changing? Otherwise nice work, approved
joyenjoyer
approved these changes
Aug 7, 2026
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Convert Jest unit tests and Cypress tests to Vitest browser tests because:
Changes in detail:
__tests__files,Truncate.cy.tsxandi18n.cy.tsxland in new files (truncate.test.tsx,getTextDirection.test.tsx) since they had no unit test file to merge into. The whole Cypress component-testing setup and its docs are then deleted.ui-iconsandui-themesresolve to their builtes/output, which roughly halves suite runtime (with a catch, see below)DateInput(the largest Cypress spec) is split across two files plus its optimized a bit for speed.vitest.setup.tsand its jsdom shims are removed as unnecessary.View.omitViewPropslogged bad error messagesFor reviewers
pnpm run test:vitest) and confirm it's green and meaningfully faster than on mastercypress/component/filesinstui-prebundle-stampinvalidation by hand: editui-themessource, rebuild it, and confirm the tests pick up the change; check the other changes invitest.config.mts