Move tests for SplitPageLayout and PageLayout to vitest #6310
Conversation
|
github-actions
bot
added
the
integration-tests: recommended
|
👋 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! |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the tests for PageLayout and SplitPageLayout components from Jest to Vitest, updating test configurations, snapshot files, and test code to be compatible with the new testing framework.
- Updates vitest.config.browser.mts to include PageLayout and SplitPageLayout test patterns
- Converts Jest snapshots to Vitest format with updated class names and test structure
- Removes Jest-specific dependencies and replaces with Vitest equivalents
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/vitest.config.browser.mts | Adds test patterns for PageLayout and SplitPageLayout components |
| packages/react/src/SplitPageLayout/SplitPageLayout.test.tsx | Updates imports and removes Jest-specific MatchMedia mock setup |
| packages/react/src/SplitPageLayout/snapshots/SplitPageLayout.test.tsx.snap | Converts Jest snapshots to Vitest format with updated class names |
| packages/react/src/PageLayout/PageLayout.test.tsx | Migrates test code from Jest to Vitest with updated mocking and browser APIs |
| packages/react/src/PageLayout/snapshots/PageLayout.test.tsx.snap | Updates snapshots with Vitest format and new CSS class names |
| import {client} from '@figma/code-connect' | ||
|
|
There was a problem hiding this comment.
This import appears to be unused in the test file. Consider removing it to keep the imports clean.
| import {client} from '@figma/code-connect' |
Copilot uses AI. Check for mistakes.
| @@ -91,7 +84,7 @@ describe('PageLayout', () => { | |||
| it.skip('shows all subcomponents by default', () => { | |||
| // Set regular viewport | |||
| act(() => { | |||
| matchMedia.useMediaQuery(viewportRanges.regular) | |||
| matchMedia(viewportRanges.regular) | |||
There was a problem hiding this comment.
This line appears to be calling matchMedia as a function without window prefix, but matchMedia should be window.matchMedia() to be consistent with line 68.
| matchMedia(viewportRanges.regular) | |
| window.matchMedia(viewportRanges.regular) |
Copilot uses AI. Check for mistakes.
| @@ -170,13 +163,18 @@ describe('PageLayout', () => { | |||
| const placeholder = await screen.findByText('Pane') | |||
| const pane = placeholder.parentNode | |||
| const initialWidth = (pane as HTMLElement).style.getPropertyValue('--pane-width') | |||
|
|
|||
| console.log('initialWidth', initialWidth) | |||
There was a problem hiding this comment.
Debug console.log statement should be removed from the test code.
| console.log('initialWidth', initialWidth) | |
| // Removed debug statement logging initialWidth |
Copilot uses AI. Check for mistakes.
| const finalWidth = (pane as HTMLElement).style.getPropertyValue('--pane-width') | ||
| console.log('finalWidth', finalWidth) |
There was a problem hiding this comment.
Debug console.log statement should be removed from the test code.
| console.log('finalWidth', finalWidth) | |
Copilot uses AI. Check for mistakes.
size-limit report 📦
|
Works on https://github.com/github/primer/issues/5336
Made necessary changes for this.