test(Switch): mode switching tests (#1825)#2000
Conversation
|
📝 WalkthroughWalkthroughA new test file is added for the ChangesSwitch Controlled/Uncontrolled Transition Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/ui/Switch/tests/Switch.controlledSwitch.test.tsx (1)
17-17: ⚡ Quick winUse role-based queries instead of
querySelectorwith non-null assertion.Line 17 and Line 41 use
container.querySelector('button')!, which is less resilient and bypasses null safety. Preferscreen.getByRole('switch')(orgetByRolefrom render result) to keep tests aligned with accessible contracts and avoid brittle selector coupling.Suggested refactor
- const { container, rerender } = render( + const { rerender } = render( <Switch.Root defaultChecked={false}> <Switch.Thumb /> </Switch.Root> ); - const button = container.querySelector('button')!; + const button = screen.getByRole('switch');- const { container, rerender } = render( + const { rerender } = render( <Switch.Root checked onCheckedChange={() => {}}> <Switch.Thumb /> </Switch.Root> ); - const button = container.querySelector('button')!; + const button = screen.getByRole('switch');Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/Switch/tests/Switch.controlledSwitch.test.tsx` at line 17, Replace the direct querySelector calls that select 'button' elements using non-null assertions with role-based queries. Instead of using container.querySelector('button')!, use screen.getByRole('switch') to query the switch element. This makes the tests more resilient to DOM structure changes and aligns with testing best practices by focusing on accessible roles rather than implementation details. Apply this change to all instances where querySelector is used with a non-null assertion for button selection in this test file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/ui/Switch/tests/Switch.controlledSwitch.test.tsx`:
- Line 17: Replace the direct querySelector calls that select 'button' elements
using non-null assertions with role-based queries. Instead of using
container.querySelector('button')!, use screen.getByRole('switch') to query the
switch element. This makes the tests more resilient to DOM structure changes and
aligns with testing best practices by focusing on accessible roles rather than
implementation details. Apply this change to all instances where querySelector
is used with a non-null assertion for button selection in this test file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a241dda-9528-48b9-910a-0d3664f97641
📒 Files selected for processing (1)
src/components/ui/Switch/tests/Switch.controlledSwitch.test.tsx
CoverageThis report compares the PR with the base branch. "Δ" shows how the PR affects each metric.
Coverage improved or stayed the same. Great job! Run |
1 participant
Summary
Addresses #1825
Test plan
Summary by CodeRabbit