Reduce enzyme usage in unit test by 25% - [1 of 3] #8616
Assignees
Labels
good first issue
Good for newcomers
No QA Needed
Apply this label when your PR does not need any QA effort.
release-7.19.0
Issue or pull request that will be included in release 7.19.0
team-mobile-platform
Comments
Cal-L
added
epic
team-mobile-platform
No QA Needed
|
Hello @Cal-L For example Enzyme uses a lowercase for components name while RTL maintains the original naming convention.What do you suggest? |
Good catch. @timi-codes What you mentioned make sense. I updated the acceptance criteria to mention that snapshots may change. |
metamaskbot
added
in-progress
needs-dev-review
github-actions
bot
removed
the
needs-dev-review
Cal-L
added a commit
that referenced
this issue
Mar 14, 2024
## **Description** Convert the list of unit tests to use react-testing-library instead of enzyme for the following files: ``` 'app/components/Views/Root/index.test.tsx', 'app/components/Views/NavigationUnitTest/TestScreen3.test.js', 'app/components/Views/NavigationUnitTest/TestScreen2.test.js', 'app/components/Views/NavigationUnitTest/TestScreen1.test.js', 'app/components/Views/ImportPrivateKeySuccess/index.test.tsx', 'app/components/Views/AccountBackupStep1B/index.test.tsx', 'app/components/UI/WebviewProgressBar/index.test.tsx', 'app/components/UI/SeedphraseModal/index.test.tsx', 'app/components/UI/PhishingModal/index.test.tsx', 'app/components/UI/FoxScreen/index.test.tsx', 'app/components/UI/FadeView/index.test.tsx', 'app/components/UI/CustomAlert/index.test.tsx', 'app/components/UI/Confetti/index.test.tsx', 'app/components/UI/Collectibles/index.test.tsx', 'app/components/UI/Button/index.test.tsx', 'app/components/UI/AnimatedSpinner/index.test.tsx', 'app/components/UI/AddCustomToken/index.test.tsx', ``` ## **Related issues** Fixes: #8616 ## **Manual testing steps** Not Applicable ## **Screenshots/Recordings** Not Applicable ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Cal Leung <cleun007@gmail.com>
26 tasks
Cal-L
added a commit
that referenced
this issue
Mar 14, 2024
## **Description** This PR merges @timi-codes 's PR that converts some of the unit tests to use react testing library. The original PR is here - #8721 Convert the list of unit tests to use react-testing-library instead of enzyme for the following files: ``` 'app/components/Views/Root/index.test.tsx', 'app/components/Views/NavigationUnitTest/TestScreen3.test.js', 'app/components/Views/NavigationUnitTest/TestScreen2.test.js', 'app/components/Views/NavigationUnitTest/TestScreen1.test.js', 'app/components/Views/ImportPrivateKeySuccess/index.test.tsx', 'app/components/Views/AccountBackupStep1B/index.test.tsx', 'app/components/UI/WebviewProgressBar/index.test.tsx', 'app/components/UI/SeedphraseModal/index.test.tsx', 'app/components/UI/PhishingModal/index.test.tsx', 'app/components/UI/FoxScreen/index.test.tsx', 'app/components/UI/FadeView/index.test.tsx', 'app/components/UI/CustomAlert/index.test.tsx', 'app/components/UI/Confetti/index.test.tsx', 'app/components/UI/Collectibles/index.test.tsx', 'app/components/UI/Button/index.test.tsx', 'app/components/UI/AnimatedSpinner/index.test.tsx', 'app/components/UI/AddCustomToken/index.test.tsx', ``` ## **Related issues** Fixes: #8616 ## **Manual testing steps** Not Applicable ## **Screenshots/Recordings** Not Applicable ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. Co-authored-by: Timi Tejumola <tejumoladavid@gmail.com>
metamaskbot
added
the
release-7.19.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What is this about?
Convert the list of unit tests to use
react-testing-libraryinstead ofenzyme. In total there are 53 files to convert. To make PRs easier to review and more manageable, this covers 17 test files.Scenario
No response
Design
No response
Technical Details
Convert these files
Threat Modeling Framework
No response
Acceptance Criteria
Unit tests for the updated files should all pass. Snapshots may be updated
Stakeholder review needed before the work gets merged
References
No response
The text was updated successfully, but these errors were encountered: