-
Notifications
You must be signed in to change notification settings - Fork 622
Migrate PointerBox tests from Jest to Vitest with computed style assertions #6407
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
|
Co-authored-by: joshblack <[email protected]>
Co-authored-by: joshblack <[email protected]>
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.
@copilot make sure to also pull in the latest changes from main
and address merge conflicts
}) | ||
|
||
it('applies the border color via "borderColor" prop for backwards compatibility', () => { | ||
expect(render(<PointerBox borderColor="danger.emphasis" />)).toMatchSnapshot() | ||
expect(render(<PointerBox borderColor="danger.emphasis" theme={theme} />)).toMatchSnapshot() |
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.
Update this test from a snapshot test to one that is checking that the border color is set correctly
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.
Updated this test to check that the border color is set correctly using computed styles instead of snapshots. Now verifies borderColor
CSS property is rgb(207, 34, 46)
for danger.emphasis
. Commit b3ddae9
}) | ||
|
||
it('applies the border color via sx prop', () => { | ||
expect(render(<PointerBox sx={{borderColor: 'danger.emphasis'}} />)).toMatchSnapshot() | ||
expect(render(<PointerBox sx={{borderColor: 'danger.emphasis'}} theme={theme} />)).toMatchSnapshot() |
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.
Update this test from a snapshot test to one that is checking that the border color is set correctly
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.
Updated this test to check that the border color is set correctly using computed styles instead of snapshots. Now verifies borderColor
CSS property is rgb(207, 34, 46)
for danger.emphasis
. Commit b3ddae9
const {container} = HTMLRender(<PointerBox />) | ||
const results = await axe.run(container) | ||
expect(results).toHaveNoViolations() | ||
expect(render(<PointerBox theme={theme} />)).toMatchSnapshot() |
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.
Remove this test
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.
Removed the basic rendering snapshot test as requested. Commit b3ddae9
<PointerBox bg="danger.emphasis" /> | ||
</ThemeProvider>, | ||
), | ||
).toMatchSnapshot() |
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.
Update this test from a snapshot test to one that is checking the background color
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.
Updated this test to check the background color using computed styles instead of snapshots. Now verifies backgroundImage
CSS property contains the expected color rgb(207, 34, 46)
for danger.emphasis
. Commit b3ddae9
}) | ||
|
||
it('applies the background color via sx prop', () => { | ||
expect(render(<PointerBox sx={{bg: 'danger.emphasis'}} />)).toMatchSnapshot() | ||
expect( |
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.
Update this test from a snapshot test to one that is checking the background color
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.
Updated this test to check the background color using computed styles instead of snapshots. Now verifies backgroundImage
CSS property contains the expected color rgb(207, 34, 46)
for danger.emphasis
. Commit b3ddae9
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.
@copilot pull in the latest changes from main and resolve merge conflicts. Also address the comments in the previous review
…heck computed styles Co-authored-by: joshblack <[email protected]>
✅ Pulled in latest changes from main and resolved merge conflicts in both config files. ✅ Addressed all review comments - removed snapshot test and converted remaining tests to check computed CSS properties. All tests passing. Commit b3ddae9 |
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 the PointerBox component tests from Jest to Vitest as part of the broader test suite migration effort, improving test reliability by replacing DOM snapshots with computed style assertions.
- Migrated test framework from Jest to Vitest with @testing-library/react
- Replaced brittle snapshot tests with computed style assertions that verify actual CSS properties
- Removed Jest-specific utilities and added ThemeProvider wrapper for styled-components context
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/react/vitest.config.browser.mts | Added PointerBox tests to Vitest browser config include list |
packages/react/src/PointerBox/snapshots/PointerBox.test.tsx.snap | Removed all Jest snapshot files as they're replaced by computed style tests |
packages/react/src/PointerBox/PointerBox.test.tsx | Complete migration from Jest to Vitest with computed style assertions replacing snapshots |
packages/react/jest.config.js | Excluded PointerBox from Jest modulePathIgnorePatterns to prevent conflicts |
👋 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 📦
|
This PR migrates the PointerBox component tests from Jest to Vitest as part of the broader test suite migration effort, replacing snapshot tests with computed style assertions for better test reliability.
Changes Made
describe
,expect
,it
) and @testing-library/react (render
)behavesAsComponent
usage (component behavior tests)checkExports
usage (module export validation)toHaveNoViolations
(accessibility testing)ThemeProvider
wrapper for tests that require styled-components theme contextborderColor
CSS property equalsrgb(207, 34, 46)
fordanger.emphasis
backgroundImage
CSS property contains the expected colorTest Coverage
All 5 tests are passing and now provide more reliable assertions:
borderColor
prop (backwards compatibility)sx
propbg
prop (backwards compatibility)sx
propbg
prop andsx
propThe migration improves test reliability by checking actual computed styles rather than DOM structure, making tests less brittle to unrelated markup changes.
Fixes #6390.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.