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

Migrate from Enzyme to React Testing Library #1197

Closed
anastasialanz opened this issue Sep 16, 2023 · 11 comments
Closed

Migrate from Enzyme to React Testing Library #1197

anastasialanz opened this issue Sep 16, 2023 · 11 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@anastasialanz
Copy link
Contributor

anastasialanz commented Sep 16, 2023

Background

In order to update Cauldron to React 18, we need to update our testing framework. Currently, Enzyme does not support React 17 or React 18.

Related resources:
https://dev.to/wojtekmaj/enzyme-is-dead-now-what-ekl
https://stackoverflow.com/questions/72029189/does-enzyme-support-react-version-18

Recommendations

✨ Switching to React Testing Library

Pros

  • Other teams already use React Testing Library and updating component tests in Cauldron would be familiar to them
  • Avoid testing implementation details and testing components the way a user would use them

Cons

  • React Testing Library is tied to a React version. Will we end up in the same situation again?

In this branch https://github.com/dequelabs/cauldron/tree/rfc--testing-library, I've used the matching React Testing Library to the React version 16.3.1 and updated the tests for the Button component as a proof of concept. We can run Enzyme and RTL together until we finish all of the test migrations.

Migration Plan

https://testing-library.com/docs/react-testing-library/migrate-from-enzyme

To ensure a successful migration, we recommend doing it incrementally by running the two test libraries side by side in the same application, porting your Enzyme tests to React Testing Library one by one. That makes it possible to migrate even large and complex applications without disrupting other business because the work can be done collaboratively and spread out over time.

  • Maintain both existing test and newly migrated RTL tests
  • New components switch to using RTL in their tests
  • Testing files will be collocated from packages/react/__test__/src/components/**/index.js to packages/react/src/components/**/*.test.tsx
  • Tests will be updated to be written in TypeScript to help catch type issues
  • Make separate issues for each component testing migration

⚠️ Switching to Playwright Component Testing

@scurker has mentioned in the past about trying out Playwright Component Testing.

Pros

  • Would allow for testing in a real browser

Cons

  • Playwright only supports React 17 and 18 so we would need to update React to use it anyway.
Screenshot 2023-09-18 at 8 21 11 AM
  • ⚠️ Playwright component testing is considered experimental. The repo says, “BEWARE This package is EXPERIMENTAL and does not respect semver.”
  • Since it's new, the team is not as familiar with this framework

Additional resources:
https://github.com/microsoft/playwright/tree/main/packages/playwright-ct-react
https://www.lambdatest.com/learning-hub/playwright-component-testing

@anastasialanz anastasialanz added rfc An issue proposing a new significant change enhancement New feature or request labels Sep 16, 2023
@scurker
Copy link
Member

scurker commented Sep 19, 2023

Thanks for putting this together!

I think switching to RTL probably makes sense as our teams are likely already familiar with it, even with some of the caveats that might exist in RTL.

A couple of questions/thoughts:

  • I'm wondering if rewriting tests should also co-locate tests next to their components, that way it would be easier to target a separate test runner (if needed). We would also need to consider what the implications are for code coverage if we did this.
  • I'm thinking if we're doing a full rewrite of tests, we should try to leverage typescript. We don't currently test the interoperability of components + typescript from within cauldron and that's a huge gap and I wonder if this is a reasonable opportunity to do so.

I don't want to scope creep here as this will already be a fairly large undertaking, but I would be curious how much effort the above might add vs the benefit.

@anastasialanz anastasialanz changed the title Migrate from Enzyme to React Testing Library or Playwright Component Testing Migrate from Enzyme to React Testing Library Sep 19, 2023
@scurker
Copy link
Member

scurker commented Sep 19, 2023

There is a list of components to do test migrations with here: #1159

@anastasialanz
Copy link
Contributor Author

anastasialanz commented Sep 19, 2023

I'm wondering if rewriting tests should also co-locate tests next to their components, that way it would be easier to target a separate test runner (if needed). We would also need to consider what the implications are for code coverage if we did this.

I'm a fan of tests in their component folders, but we can also leave the tests in the __tests__ folder and run both test runners side by side without issue. I can test this in a different branch based on my button test branch and see what the code coverage implications would be.

Related, if a component test file has 5 tests and we update functionality for that component, should the new test be written using RTL in the same file or should the entire file be converted to RTL in addition to the new test? This could allow someone to only focus on the new code and leave other test rewrites for a separate PR.

I'm thinking if we're doing a full rewrite of tests, we should try to leverage typescript. We don't currently test the interoperability of components + typescript from within cauldron and that's a huge gap and I wonder if this is a reasonable opportunity to do so.

Are you saying that when I want to rewrite tests for a component to RTL I also need to add TypeScript types for that component or components?

@scurker
Copy link
Member

scurker commented Sep 20, 2023

Related, if a component test file has 5 tests and we update functionality for that component, should the new test be written using RTL in the same file or should the entire file be converted to RTL in addition to the new test? This could allow someone to only focus on the new code and leave other test rewrites for a separate PR.

I think my vote would be to convert the test to use RTL. I do realize that will tack on additional work for someone making a small change to a component, but without fulltime dedicated resources to Cauldron, I think we need to try to make gains wherever we can.

Are you saying that when I want to rewrite tests for a component to RTL I also need to add TypeScript types for that component or components?

The test itself should be written in typescript, hopefully this wouldn't require changing any component typescript types and might expose issues where something was typed incorrectly.

@scurker
Copy link
Member

scurker commented Sep 20, 2023

Thinking out loud: we did a conversion of all the documentation from custom React documentation to MDX starting in mid-March, with the final conversion completed mid-August, so roughly ~5 months. Assuming that converting tests takes roughly the same amount of time as documentation, it is possible that we could have all tests converted over to RTL by end of Q1 2024 if we were to start in October.

If we could borrow some resources from teams, this could go faster - just trying to get a rough time frame out there of what kind of scope we're looking at.

@anastasialanz
Copy link
Contributor Author

Assuming that converting tests takes roughly the same amount of time as documentation

I'm not sure if this is a fair comparison but I looked at some of the largest test files and compared it to the lines of code in the docs file. Looking through all of the test files, there are more test files under 100 lines that would be quicker to migrate, but I can imagine the ones below taking more effort.

Component Name Lines of Tests Lines of Docs
OptionsMenu 550 99
Pagination 327 161
Accordion 292 99
Toast 272 191
ExpandCollapsePanel 267 114
TwoColumnPanel 234 317

@scurker
Copy link
Member

scurker commented Sep 20, 2023

I'm not sure if this is a fair comparison

Maybe not, I'm just trying to put some ballpark figure out there to work with. We can pad it accordingly once I can figure out if there's additional resources we can lean on.

@scurker
Copy link
Member

scurker commented Nov 1, 2023

👍 from me!

@JoshMK
Copy link
Contributor

JoshMK commented Nov 1, 2023

👍

1 similar comment
@DelishusCake
Copy link
Contributor

👍

@anastasialanz anastasialanz removed the rfc An issue proposing a new significant change label Nov 2, 2023
@scurker scurker added this to the Q4 2023 milestone Nov 2, 2023
@scurker scurker modified the milestones: Q4 2023, Q1 2024 Jan 2, 2024
anastasialanz added a commit that referenced this issue Jan 10, 2024
Since we eventually wanted to move to the latest version of React, we switched to using React Testing Library. We'll be migrating each component's test file from Enzyme to React Testing Library. This process has been started with the Button component.

Issues #1197 #1159
@anastasialanz
Copy link
Contributor Author

Closing this issue since the initial setup has been completed. We'll track migrating each component in #1159.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants