Skip to content

fix(#8617): Reduce enzyme usage in unit test by 25% - [2/3]#8823

Merged
Cal-L merged 1 commit into
MetaMask:fix/8617-reduce-enzyme-by-25-2/3from
timi-codes:reduce-enzymes-usage-2/3
Mar 19, 2024
Merged

fix(#8617): Reduce enzyme usage in unit test by 25% - [2/3]#8823
Cal-L merged 1 commit into
MetaMask:fix/8617-reduce-enzyme-by-25-2/3from
timi-codes:reduce-enzymes-usage-2/3

Conversation

@timi-codes

@timi-codes timi-codes commented Mar 2, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Convert the list of unit tests to use react-testing-library instead of enzyme for the following files:

'app/components/UI/ActionView/index.test.tsx',
'app/components/UI/ActionModal/index.test.tsx',
'app/components/Views/confirmations/components/CustomNonce/index.test.tsx',
'app/components/Views/confirmations/SendFlow/ErrorMessage/index.test.tsx',
'app/components/Views/Settings/Contacts/AmbiguousAddressSheet/AmbiguousAddressSheet.test.tsx',
'app/components/Views/confirmations/components/Approval/ApprovalFlowLoader/index.test.tsx',
'app/components/UI/ImageIcon/index.test.tsx',
'app/components/UI/FadeOutOverlay/index.test.tsx',
'app/components/UI/AssetIcon/index.test.tsx',
'app/components/UI/TimeEstimateInfoModal/index.test.tsx',
'app/components/UI/Tabs/index.test.tsx',
'app/components/UI/ModalNavbarTitle/index.test.tsx',
'app/components/Views/WalletConnectSessions/index.test.tsx',
'app/component-library/components/Tags/Tag/Tag.test.tsx',
'app/components/UI/SwitchCustomNetwork/index.test.tsx',
'app/component-library/components/Avatars/Avatar/variants/AvatarAccount/AvatarAccount.test.tsx',
'app/components/Views/ImportPrivateKey/index.test.tsx'

Related issues

Fixes: #8617

Manual testing steps

Not Applicable

Screenshots/Recordings

Not Applicable

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • 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 format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

@github-actions

github-actions Bot commented Mar 2, 2024

Copy link
Copy Markdown
Contributor

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.

@timi-codes timi-codes marked this pull request as ready for review March 2, 2024 04:52
@timi-codes timi-codes requested review from a team as code owners March 2, 2024 04:52
@timi-codes

timi-codes commented Mar 5, 2024

Copy link
Copy Markdown
Contributor Author

@Cal-L This is also ready for review

@timi-codes timi-codes mentioned this pull request Mar 12, 2024
Merged
13 tasks
Cal-L
Cal-L reviewed Mar 18, 2024
View reviewed changes

@Cal-L Cal-L left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timi-codes Left some comments. Let's also resolve the conflicts

Comment thread app/components/Views/Settings/Contacts/AmbiguousAddressSheet/AmbiguousAddressSheet.test.tsx Outdated
Comment thread app/components/Views/WalletConnectSessions/index.test.tsx Outdated
Comment thread app/components/Views/WalletConnectSessions/index.test.tsx Outdated
Comment thread app/components/Views/WalletConnectSessions/index.test.tsx Outdated
Comment thread app/components/Views/WalletConnectSessions/index.test.tsx Outdated
Comment thread app/components/Views/WalletConnectSessions/index.test.tsx Outdated
@timi-codes timi-codes requested a review from a team March 19, 2024 06:12
@timi-codes

timi-codes commented Mar 19, 2024

Copy link
Copy Markdown
Contributor Author

@Cal-L Thanks for the feedback. I think /@kylanhurt already worked on the same files. Reason for the conflict. However, I already pulled his changes into this branch. Now we have just a few changes

@timi-codes timi-codes requested a review from Cal-L March 19, 2024 15:11
@Cal-L Cal-L changed the base branch from main to fix/8617-reduce-enzyme-by-25-2/3 March 19, 2024 20:47
Cal-L
Cal-L approved these changes Mar 19, 2024
View reviewed changes

@Cal-L Cal-L left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Cal-L Cal-L merged commit 59b47b3 into MetaMask:fix/8617-reduce-enzyme-by-25-2/3 Mar 19, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Cal-L Cal-L Cal-L approved these changes

Assignees

No one assigned

Labels

external-contributor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Reduce enzyme usage in unit test by 25% - [2 of 3]

3 participants

@timi-codes @Cal-L @metamaskbot