-
Notifications
You must be signed in to change notification settings - Fork 608
test(project): add support for vitest browser tests #5989
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
base: main
Are you sure you want to change the base?
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
Adds incremental Vitest support and configures browser-based testing while transitioning tests from Jest to Vitest.
- Updates build and test configs to include Vitest files and ignore migrated Jest directories
- Migrates most component and utility tests from
jest.*
tovitest
imports andvi.*
calls, including snapshot updates - Adds Vitest setup, PostCSS Vitest config, and a new CI step to run Vitest tests
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/react/tsconfig.build.json | Includes vitest.config.mts and new config files in the build input |
packages/react/src/Stack/tests/StackItem.test.tsx | Imports Vitest globals and replaces jest.fn with vi.fn |
packages/react/src/Stack/tests/Stack.test.tsx | Imports Vitest globals and replaces jest.fn with vi.fn |
packages/react/src/FeatureFlags/tests/FeatureFlags.test.tsx | Adds Vitest imports for browser-based FeatureFlags tests |
packages/react/src/DataTable/tests/sorting.test.ts | Imports Vitest globals for sorting tests |
packages/react/src/DataTable/tests/Table.test.tsx | Removes Jest skip and migrates to Vitest globals |
packages/react/src/DataTable/tests/Pagination.test.tsx | Adds browser context viewport setup and migrates to vi.* |
packages/react/src/DataTable/tests/ErrorDialog.test.tsx | Migrates from jest.fn to vi.fn in ErrorDialog tests |
packages/react/src/DataTable/tests/DataTable.test.tsx | Migrates jest.spyOn and jest.fn calls to Vitest equivalents |
packages/react/src/Banner/snapshots/Banner.test.tsx.snap | Updates snapshot header and snapshot formatting for Vitest |
packages/react/src/Banner/Banner.test.tsx | Migrates from Jest to Vitest imports and calls, removes prior Jest error suppression block |
packages/react/jest.config.js | Adds modulePathIgnorePatterns to skip test dirs now under Vitest |
packages/react/config/vitest/setup.ts | Sets up global cleanup and jest-dom matchers for Vitest |
packages/postcss-preset-primer/vitest.config.ts | Adds Vitest configuration for the PostCSS preset |
packages/postcss-preset-primer/tsconfig.json | Includes vitest.config.ts in TypeScript compilation |
packages/postcss-preset-primer/src/tests/preset.test.ts | Migrates from Jest environment directive to Vitest imports |
packages/postcss-preset-primer/src/tests/snapshots/preset.test.ts.snap | Updates snapshot headers and formatting for Vitest |
packages/postcss-preset-primer/jest.config.cjs | Removes legacy Jest config now replaced by Vitest |
package.json | Adds vitest and @vitest/browser dependencies |
.github/workflows/ci.yml | Adds a CI step to run Vitest tests in a browser container |
Comments suppressed due to low confidence (3)
packages/react/src/Banner/Banner.test.tsx:7
- The prior Jest-based
console.error
suppression for jsdom CSS parse errors was removed, which may cause test failures or noise. Reintroduce a mock or filter aroundconsole.error
to suppress the knownCould not parse CSS stylesheet
errors.
describe('Banner', () => {
packages/react/src/DataTable/tests/sorting.test.ts:4
- [nitpick] The constant
Second
is PascalCase and singular. Rename toSECOND
or a more descriptive name likeMILLISECONDS_IN_SECOND
to follow constant naming conventions.
const Second = 1000
.github/workflows/ci.yml:71
- The Vitest CI step uses a Playwright Docker image without mounting the workspace, so the tests and dependencies won’t be available. Consider running Vitest directly on the runner (e.g.,
run: npx vitest
) or correctly mounting the repository inside the container.
uses: docker://mcr.microsoft.com/playwright:v1.51.0-jammy
import React from 'react' | ||
import {Pagination} from '../Pagination' | ||
import {render, screen} from '@testing-library/react' | ||
import userEvent from '@testing-library/user-event' | ||
|
||
describe('Table.Pagination', () => { | ||
beforeEach(async () => { | ||
await page.viewport(1400, 728) |
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.
Playwright’s API no longer uses page.viewport
. Replace with await page.setViewportSize({ width: 1400, height: 728 })
to ensure the viewport is set correctly.
await page.viewport(1400, 728) | |
await page.setViewportSize({ width: 1400, height: 728 }) |
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.
Curious to hear what you all think! Would you support a gradual transition over to Vitest? Would you prefer sticking with Jest? Let me know!
I think transitioning makes sense if anything for the styling upgrades alone!
Add support for vitest browser tests to Primer React.
This change introduces a strategy for how we could incrementally move our tests in Jest over to Vitest. We can configure Vitest to include specific files and exclude them from Jest in order to gradually transition from one test runner to another.
One notable challenge in doing a wholesale migration is that there are currently certain tests that require more work to update to Vitest. In addition, our test helpers (like for matchers and global stubs) can also make certain tests difficult to migrate.
However, a large number of tests can be updated through the following steps:
vitest
jest.*
calls withvi.*
callsVitest is also configured to run tests within the context of a browser. This means that our styles are brought into the page and we can rely on them being present when writing our tests.
Curious to hear what you all think! Would you support a gradual transition over to Vitest? Would you prefer sticking with Jest? Let me know!