Skip to content
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: replace Enzyme with React Testing Library #12539

Conversation

tusharnagar17
Copy link

Description

Replaced Enzyme usage with React Testing Library in the following files:

  • Test successfully written for these file:
app/components/Views/confirmations/components/TransactionReview/TransactionReviewSummary/index.test.tsx
app/components/Views/confirmations/components/TransactionReview/TransactionReviewDetailsCard/index.test.js
app/components/Views/Settings/Contacts/ContactForm/index.test.tsx
app/component-library/components/Avatars/Avatar/variants/AvatarFavicon/AvatarFavicon.test.tsx
app/component-library/components/Form/TextField/foundation/Input/Input.test.tsx
app/components/Views/confirmations/components/SignatureRequest/ExpandedMessage/index.test.tsx
app/components/Views/confirmations/components/SignatureRequest/Root/Root.test.tsx
app/component-library/components/Buttons/Button/foundation/ButtonBase/ButtonBase.test.tsx
app/component-library/components/Accordions/Accordion/foundation/AccordionHeader/AccordionHeader.test.tsx
app/component-library/components/Badges/Badge/foundation/BadgeBase/BadgeBase.test.tsx
app/component-library/components/Badges/Badge/variants/BadgeStatus/BadgeStatus.test.tsx
app/component-library/components/Avatars/Avatar/variants/AvatarNetwork/AvatarNetwork.test.tsx
app/component-library/components/Avatars/Avatar/variants/AvatarToken/AvatarToken.test.tsx
app/component-library/components/Avatars/Avatar/foundation/AvatarBase/AvatarBase.test.tsx
  • Test for the below file is not completed getting the the same error for each file below
  • Error : TypeError: _reactNative.DeviceEventEmitter.removeListener is not a function
app/components/Views/confirmations/components/TransactionReview/TransactionReviewInformation/index.test.tsx
app/components/Views/confirmations/Approval/components/TransactionEditor/index.test.tsx
app/components/Views/confirmations/components/TransactionReview/TransactionReviewData/index.test.tsx
app/components/Views/confirmations/ApproveView/Approve/index.test.tsx
app/components/Views/Settings/NetworksSettings/NetworkSettings/index.test.tsx
app/components/Views/confirmations/SendFlow/components/CustomNonceModal/index.test.tsx

Related issues

Fixes: #12300

Manual testing steps

yarn test filename.test.tsx -t "DescribeText ItText"

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

@tusharnagar17 tusharnagar17 requested review from a team as code owners December 4, 2024 07:00
Copy link
Contributor

github-actions bot commented Dec 4, 2024

CLA Signature Action:

Thank you for your submission, we really appreciate it. We ask that you read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:

I have read the CLA Document and I hereby sign the CLA

By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to this repository.

0 out of 1 committers have signed the CLA.
@tusharnagar17


Enzyme.configure({ adapter: new Adapter() });
global.TextEncoder = TextEncoder;
global.TextDecoder = TextDecoder;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to explain the reason for modifying these global variables? (TextEncoder & TextDecoder)

Copy link
Author

Choose a reason for hiding this comment

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

  • Enzyme rarely needs TextEncoder and TextDecoder as it's focused on React component testing, which typically doesn’t involve these APIs. With a properly configured jsdom or browser-like environment, they might already be polyfilled. Most Enzyme use cases don’t rely on text encoding/decoding.

  • But in case of RNTL, Jest needs TextEncoder and TextDecoder because it runs in a Node.js environment, which doesn’t include these browser-native APIs. Even with jsdom, these specific features aren't implemented. Adding them globally ensures compatibility with libraries or code that depend on text encoding/decoding.

  • Without using global.TextEncoder and global.Decoder getting this particular issue
    https://stackoverflow.com/questions/68468203/why-am-i-getting-textencoder-is-not-defined-in-jest

@MarioAslau
Copy link
Contributor

Hi @tusharnagar17 ,

Thank you for taking the time to help us. I have left a comment on your PR.

Could you please complete the CLA Signature Action so that we can accept your contributions?

Additionally, as per our contributor guidelines, it would be very helpful if you could break this PR into smaller, more manageable PRs. This will make it easier for us to review and test your contributions, as the current PR does not adhere to these guidelines.

In case of any questions, don't hesitate to reach out.

Thank you for your understanding!

@tusharnagar17
Copy link
Author

Hi @MarioAslau,

Thank you for the feedback.
I will complete the CLA Signature Action and submit smaller, more manageable PRs as per the guidelines.

Kindly disregard this current pull request, and I’ll submit a new one shortly.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2024
@tusharnagar17 tusharnagar17 deleted the refractor/replace-enzyme-with-rtl-5 branch December 16, 2024 06:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Remove Enzyme usage (5/5)
4 participants