-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
test(Migration to RTL): Refactor ChartTable.test.tsx from Enzyme to RTL #27429
test(Migration to RTL): Refactor ChartTable.test.tsx from Enzyme to RTL #27429
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #27429 +/- ##
=======================================
Coverage 67.36% 67.37%
=======================================
Files 1909 1909
Lines 74670 74678 +8
Branches 8324 8325 +1
=======================================
+ Hits 50304 50311 +7
- Misses 22315 22317 +2
+ Partials 2051 2050 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@justinpark could you take a look at this one when you have a moment? |
handler({} as any); | ||
} | ||
await renderChartTable(mockedProps); | ||
fireEvent.click(screen.getByText(/favorite/i)); |
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.
Let's use userEvent
instead as suggested in the wiki
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.
@justinpark curious what should be the go-to usage, fireEvent
or userEvent
, as you mentioned we should save userEvent
for special cases but this seems to go against the RTL wiki and docs
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.
@justinpark sent this article . Switching to userEvent
46d440f
to
972a78b
Compare
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.
LGTM
SUMMARY
Replace enzyme with RTL functions. Refactor the tests accordingly. Added a test for the
mine
tab.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
npm test ChartTable.test.tsx
ADDITIONAL INFORMATION