-
Notifications
You must be signed in to change notification settings - Fork 609
Update tests from Jest to Vitest #6112
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 continues the migration of component tests from Jest to Vitest by updating configuration files and rewriting several test suites using Vitest’s APIs and React Testing Library.
- Added Vitest test globs for newly migrated components in
vitest.config.mts
- Rewrote tests for Textarea, ScrollableRegion, RelativeTime, RadioGroup, Radio, ProgressBar to use Vitest and RTL
- Updated snapshots and test setup for React 18’s
act
environment
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
packages/react/vitest.config.mts | Added test globs for Radio, RadioGroup, RelativeTime, ProgressBar, ScrollableRegion, Textarea |
packages/react/src/Textarea/Textarea.test.tsx | Replaced Jest utils with Vitest imports and RTL patterns |
packages/react/src/ScrollableRegion/ScrollableRegion.test.tsx | Switched global to window ResizeObserver and updated style assertions |
packages/react/src/RelativeTime/RelativeTime.test.tsx | Removed Jest helpers, now uses Vitest and RTL |
packages/react/src/RadioGroup/RadioGroup.test.tsx | Replaced Jest mocks with vi and removed legacy helpers |
packages/react/src/Radio/Radio.test.tsx | Switched from Jest to Vitest for mocks and assertions |
packages/react/src/ProgressBar/snapshots/ProgressBar.test.tsx.snap | Updated snapshot header and output for Vitest |
packages/react/src/ProgressBar/ProgressBar.test.tsx | Replaced Jest helpers with Vitest and RTL rendering |
packages/react/jest.config.js | Added paths for newly migrated test directories |
packages/react/config/vitest/setup.ts | Enabled IS_REACT_ACT_ENVIRONMENT for React 18 tests |
Comments suppressed due to low confidence (1)
packages/react/src/RelativeTime/RelativeTime.test.tsx:3
- Accessibility tests using axe were removed during this migration. Consider re-adding an axe-based test (e.g.
expect(await axe.run(container)).toHaveNoViolations()
) to maintain accessibility coverage for .
import {render} from '@testing-library/react'
checkExports('RadioGroup', { | ||
default: RadioGroup, | ||
RadioGroupContext, | ||
vi.clearAllMocks() |
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.
The afterAll block clears mock calls but does not restore the original console.warn implementation. Consider using vi.restoreAllMocks()
or calling mockRestore()
on the spy to prevent stubbing console.warn in other tests.
vi.clearAllMocks() | |
vi.restoreAllMocks() |
Copilot uses AI. Check for mistakes.
{ | ||
"asFragment": [Function], | ||
"baseElement": <body> | ||
|
||
|
||
|
||
exports[`ProgressBar respects the "progress" prop 1`] = ` | ||
.c0 { | ||
background-color: var(--bgColor-success-emphasis,var(--color-success-emphasis,#1f883d)); | ||
} | ||
|
||
<span | ||
className="ProgressBarContainer" | ||
data-progress-bar-size="default" | ||
data-progress-display="block" | ||
> | ||
<span | ||
aria-label="Upload test.png" | ||
aria-valuemax={100} | ||
aria-valuemin={0} | ||
aria-valuenow={80} | ||
className="c0 ProgressBarItem" | ||
role="progressbar" | ||
style={ | ||
{ | ||
"--progress-bg": "var(--bgColor-success-emphasis)", | ||
"--progress-width": "80%", | ||
} | ||
} | ||
/> | ||
</span> | ||
<div> | ||
<span | ||
class="_ProgressBarContainer_1bos7_27" | ||
data-progress-bar-size="default" | ||
data-progress-display="block" | ||
> | ||
<span | ||
aria-label="Upload test.png" | ||
aria-valuemax="100" | ||
aria-valuemin="0" | ||
aria-valuenow="80" | ||
class="sc-aXZVg lfjCwU _ProgressBarItem_1bos7_11" | ||
role="progressbar" | ||
style="--progress-width: 80%; --progress-bg: var(--bgColor-success-emphasis);" | ||
/> | ||
</span> | ||
</div> | ||
</body>, | ||
"container": <div> |
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.
[nitpick] The snapshot includes the full Testing Library API, making it very verbose. Consider switching to a DOM-only snapshot (e.g. via asFragment()
or toJSON()
) to keep snapshots concise and easier to review.
Copilot uses AI. Check for mistakes.
expect(screen.getByTestId('container')).toHaveStyle('overflow: auto') | ||
expect(screen.getByTestId('container')).toHaveStyle('position: relative') |
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.
The toHaveStyle
matcher only checks inline style attributes, but overflow
and position
come from CSS. Use getComputedStyle(container).overflow
(and similarly for position
) to assert these style rules reliably.
expect(screen.getByTestId('container')).toHaveStyle('overflow: auto') | |
expect(screen.getByTestId('container')).toHaveStyle('position: relative') | |
const containerStyles = getComputedStyle(screen.getByTestId('container')); | |
expect(containerStyles.overflow).toBe('auto'); | |
expect(containerStyles.position).toBe('relative'); |
Copilot uses AI. Check for mistakes.
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.
🙌🏻
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Pull Request is not mergeable
Continue our refactor of tests from Jest to Vitest.
Changelog
New
Changed
Removed