refactor: update tests from Jest to Vitest #6227
Conversation
|
github-actions
bot
added
staff
|
👋 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 📦
|
joshblack
requested review from
TylerJDev
and removed request for
jonrohan and
francinelucca
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the test suite from Jest to Vitest across several packages, updating test configs, test files, and story imports to align with Vitest’s APIs and eliminate legacy Jest patterns.
- Updated Vitest configurations to include React plugin, global defines, and expanded test globs.
- Migrated individual component tests (e.g., Box, Flash, InlineMessage, Octicon, Pagination, internal/utils, utils, storybook) from Jest to Vitest, removing snapshots and axe checks.
- Fixed import paths for
ReactRouterLikeLinkin stories and examples.
Reviewed Changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/vitest.config.mts | Added React plugin, defined __DEV__, and expanded test include |
| packages/react/vitest.config.browser.mts | Added test globs for Box, Flash, InlineMessage, Octicon, Pagination, internal/utils, and utils |
| packages/react/src/stories/deprecated/ActionList.stories.tsx | Updated ReactRouterLikeLink import path |
| packages/react/src/internal/utils/tests/toggleSxComponent.test.tsx | Migrated to Vitest imports and updated style assertions |
| packages/react/src/internal/utils/tests/toggleStyledComponent.test.tsx | Migrated to Vitest imports and updated style assertions |
| packages/react/src/internal/utils/tests/hasInteractiveNodes.test.ts | Added Vitest imports |
| packages/react/src/internal/utils/tests/getResponsiveAttributes.test.ts | Added Vitest imports |
| packages/react/src/tests/storybook.test.tsx | Migrated to Vitest, replaced require with dynamic import, and added top-level await |
| packages/react/src/tests/Pagination/Pagination.test.tsx | Removed old Jest-based Pagination test |
| packages/react/src/Pagination/PaginationModel.test.tsx | Migrated to Vitest imports and updated model import path |
| packages/react/src/Pagination/Pagination.test.tsx | Added new Vitest-based Pagination test |
| packages/react/src/Pagination/Pagination.features.stories.tsx | Fixed ReactRouterLikeLink import path |
| packages/react/src/Octicon/Octicon.test.tsx | Migrated to Vitest, removed legacy utils/axe tests, added new assertions |
| packages/react/src/NavList/NavList.test.tsx | Fixed ReactRouterLikeLink import path |
| packages/react/src/NavList/NavList.stories.tsx | Fixed ReactRouterLikeLink import path |
| packages/react/src/InlineMessage/InlineMessage.test.tsx | Migrated to Vitest imports |
| packages/react/src/Flash/tests/Flash.test.tsx | Migrated to Vitest imports |
| packages/react/src/Button/LinkButton.features.stories.tsx | Fixed ReactRouterLikeLink import path |
| packages/react/src/Box/snapshots/Box.test.tsx.snap | Removed Jest snapshot file for Box |
| packages/react/src/Box/Box.test.tsx | Introduced comprehensive Vitest tests for Box styling props |
| packages/react/src/ActionList/ActionList.examples.stories.tsx | Fixed ReactRouterLikeLink import path |
| packages/react/jest.config.js | Updated Jest config to include newly migrated tests |
Comments suppressed due to low confidence (1)
packages/react/src/Box/Box.test.tsx:1
- Accessibility tests using
axe-corehave been removed. It’s helpful to include an aXe accessibility check to catch regressions in component markup.
import {render, screen} from '@testing-library/react'
| }) | ||
| }) | ||
|
|
||
| function getClassName(className: string): string { |
There was a problem hiding this comment.
The helper getClassName assumes two classes are always generated and picks the second one, which can be brittle if class generation changes. Consider using a regex or filtering logic to reliably select the correct class, or using a utility to extract the generated class.
Copilot uses AI. Check for mistakes.
| function getCSSRuleForClass(className: string, mediaText?: string): CSSStyleRule { | ||
| for (const sheet of document.styleSheets) { | ||
| for (const rule of sheet.cssRules) { | ||
| if (mediaText) { | ||
| if (rule instanceof CSSMediaRule && rule.media.mediaText === mediaText) { | ||
| for (const innerRule of rule.cssRules) { | ||
| if (innerRule instanceof CSSStyleRule && innerRule.selectorText === `.${className}`) { | ||
| return innerRule | ||
| } | ||
| } | ||
| } | ||
| } else if (rule instanceof CSSStyleRule) { | ||
| if (rule.selectorText === `.${className}`) { | ||
| return rule | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| throw new Error(`CSS rule for class "${className}" not found`) |
There was a problem hiding this comment.
Searching through all document.styleSheets and their rules can be expensive and might slow down tests. Consider caching the target stylesheet or limiting the search scope to improve test performance.
| function getCSSRuleForClass(className: string, mediaText?: string): CSSStyleRule { | |
| for (const sheet of document.styleSheets) { | |
| for (const rule of sheet.cssRules) { | |
| if (mediaText) { | |
| if (rule instanceof CSSMediaRule && rule.media.mediaText === mediaText) { | |
| for (const innerRule of rule.cssRules) { | |
| if (innerRule instanceof CSSStyleRule && innerRule.selectorText === `.${className}`) { | |
| return innerRule | |
| } | |
| } | |
| } | |
| } else if (rule instanceof CSSStyleRule) { | |
| if (rule.selectorText === `.${className}`) { | |
| return rule | |
| } | |
| } | |
| } | |
| } | |
| throw new Error(`CSS rule for class "${className}" not found`) | |
| const cssRuleCache = new Map<string, CSSStyleRule>(); | |
| function getCSSRuleForClass(className: string, mediaText?: string): CSSStyleRule { | |
| const cacheKey = mediaText ? `${className}|${mediaText}` : className; | |
| if (cssRuleCache.has(cacheKey)) { | |
| return cssRuleCache.get(cacheKey)!; | |
| } | |
| for (const sheet of document.styleSheets) { | |
| for (const rule of sheet.cssRules) { | |
| if (mediaText) { | |
| if (rule instanceof CSSMediaRule && rule.media.mediaText === mediaText) { | |
| for (const innerRule of rule.cssRules) { | |
| if (innerRule instanceof CSSStyleRule && innerRule.selectorText === `.${className}`) { | |
| cssRuleCache.set(cacheKey, innerRule); | |
| return innerRule; | |
| } | |
| } | |
| } | |
| } else if (rule instanceof CSSStyleRule) { | |
| if (rule.selectorText === `.${className}`) { | |
| cssRuleCache.set(cacheKey, rule); | |
| return rule; | |
| } | |
| } | |
| } | |
| } | |
| throw new Error(`CSS rule for class "${className}" not found`); |
Copilot uses AI. Check for mistakes.
Part of https://github.com/github/primer/issues/5336
Changelog
New
Changed
Update the following tests from Jest to Vitest:
Removed