-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
chore: Migrate a batch of unit tests away from enzyme #8839
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8839 +/- ##
==========================================
+ Coverage 41.61% 43.20% +1.58%
==========================================
Files 1269 1270 +1
Lines 30869 30887 +18
Branches 3086 3083 -3
==========================================
+ Hits 12846 13344 +498
+ Misses 17246 16769 -477
+ Partials 777 774 -3 ☔ View full report in Codecov by Sentry. |
WIP fixing unit tests Fixed Send component snapshot' Provisional completion of first batch migrate from enzyme to testing-library Revert some this.context conditionals that were breaking unit tests
2a429d7
to
6e1c9d6
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.
Let's address the lint failure in the checks
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.
Left some comments. Looks like a file was missed as well
app/component-library/components/Cards/Card/Card.test.tsx
...components/UI/EnableAutomaticSecurityChecksModal/EnableAutomaticSecurityChecksModal.test.tsx
Outdated
Show resolved
Hide resolved
app/components/Views/confirmations/components/TransactionReview/index.js
Show resolved
Hide resolved
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 test for
toJSON()
for tests where it's testing wrapper directly - Unit tests are failing
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.
Left some comments
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, great work!
|
Description
Works on #8804
This PR is in line with the mobile platform's team to reduce usage of Enzyme as a testing library. The following tests were modified:
Related issues
Fixes: #8804
Manual testing steps
yarn test:unit
Screenshots/Recordings
Pre-merge author checklist
Pre-merge reviewer checklist