-
Notifications
You must be signed in to change notification settings - Fork 646
Migrate ActionMenu tests to vitest #6435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates ActionMenu tests from Jest to Vitest as part of the broader testing framework migration. The migration revealed focus handling issues that were previously suppressed in Jest tests, requiring some tests to be skipped until the underlying focus trap bug is resolved.
- Migrates ActionMenu test suite from Jest to Vitest
- Updates mock functions from
jest.fn()tovi.fn() - Skips two tests that expose focus handling bugs and adjusts one test to work around the issue
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/react/vitest.config.browser.mts |
Adds ActionMenu test files to Vitest browser configuration |
packages/react/src/ActionMenu/ActionMenu.test.tsx |
Migrates test suite from Jest to Vitest with necessary adjustments |
packages/react/jest.config.js |
Excludes ActionMenu directory from Jest configuration |
6a0125c to
94e09db
Compare
94e09db to
77c1ca2
Compare
Closes #6383
Migrates the
ActionMenutests from jest to vitest.In converting the tests, they highlighted an issue with the focus handling (#6434) that were unexpectedly being suppressed in the jest tests. There are 2 tests that I explicitly skipped b/c they were directly testing this behavior, and 1 that needed a slightly different setup procedure to account for the bug. These should be removed when #6434 is addressed
Rollout strategy