Skip to content

Add Migrate from Enzyme page #568

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

Merged
merged 7 commits into from
Sep 6, 2020
Merged

Conversation

m98
Copy link
Member

@m98 m98 commented Aug 19, 2020

Hello everyone!

As discussed earlier on #563, we realized a new page for those developers who have experience with Enzyme is needed, for at least the following purposes:

  • Why use React Testing Library instead of Enzyme?
  • How to use it? (installation, basic usage)
  • some helpful link, and explanation to help them get started with the RTL's queries

I did not go into detail on the queries, since it has a complete documentation page itself, and it would be hard to keep two pages updated (I mean if going into detail on this page) instead I just explained the difference and some helpful link to help them get started with the RTL.

Please note that I'm not a native English speaker, so, grammar problem is expected even though I tried to check it with some grammar checker software.

Thanks a lot
Mention: @kentcdodds

closes #563

@kentcdodds
Copy link
Member

Thank you for this! I haven't the time to read it yet, but I want you to know that we've taken notice of the PR and we'll look at it as soon as we're able.

@m98
Copy link
Member Author

m98 commented Aug 23, 2020

Thank you, Kent.

If there is anything that needs to be changed, please let me know

Copy link
Collaborator

@alexkrolick alexkrolick left a comment

Choose a reason for hiding this comment

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

I think this page can trim down on the installation and usage sections (we have other doc pages for that), and focus on migrating specific testing patterns.

  • Events - what you have here about user-event is good; may want to cover fireEvent and discuss differences.
  • Finding elements
    • Here's a migration strategy for wrapper.find(ComponentName):
      • Add a testid to the component => Change .find() to use the testid attribute (might now have issues with hostNodes/ReactElementWrappers both showing up) => migrate to RTL.
    • Discuss how tests for component structure can be overconstraining.
    • Show how you can use getByRole, label, etc. instead of testids if your components are ARIA-compliant.
  • Checking prop values in components
    • Address why this is not a pattern commonly used in RTL (validate behavior instead). Show how you can use mocks or spies to do it anyway.
  • Checking component state
    • Discuss how you can validate the render result instead
  • Shallow render
    • Show how you can mock a troublesome component (like a 3rd party lib) that you want to treat as a passthrough/noop componentt in the test env
  • Triggering class methods in tests
    • Discuss how this is not recommended because it overspecifies the component (can you rewrite it with Hooks and still have the tests pass)?
  • Use of act()
    • Not usually necessary in RTL due to wrapping behind the scenes
  • Use of wrapper.update()
    • Not necessary since there is no wrapper
    • Show await findBy* and await waitFor(callback) for async interactions
  • Naming conventions
  • Don't call the value returned by render wrapper; mention use of screen to avoid using that value at all, except for {rerender}
  • Updating props
    • Show use of rerender method
  • Updating state
    • Don't do it
  • Working with Redux/Context
    • Migrating wrapped/container components
    • Testing "connected" and "pure" components in an integrated and non-integrated fashion
  • Differences in queries
    • Enzyme .find() is like jQuery - it can return an array/NodeList. Show how this creates extra handling in tests (like nodes[0]) and allows errors to get farther in the test while throwing non-descriptive error messages. Show getBy and findBy + *AllBy as an alternative - they throw immediately with a detailed DOM printout and you can specify if you want 1 or multiple values.

@m98
Copy link
Member Author

m98 commented Aug 25, 2020

Thanks for your comments

I was waiting for others to write about their ideas, so I can start working on this page again.

@kentcdodds do you agree with all of what @alexkrolick said? Thoughts?

@kentcdodds
Copy link
Member

Yes, I agree with Alex

Co-authored-by: Nick McCurdy <[email protected]>
@m98
Copy link
Member Author

m98 commented Aug 27, 2020

@alexkrolick Thanks for all your good suggestions.
I'm working on it, I just have a couple of questions.

Question 1

Add a testid to the component => Change .find() to use the testid attribute (might now have issues with hostNodes/ReactElementWrappers both showing up) => migrate to RTL.

I don't understand about the example you want me to provide, can you please clarify?

This is what I understand:

  1. a component is needed
  2. A test that users wrapper.find should be used
  3. Now the test has to be changed to use testid (why?)
  4. I did not understand hostNodes/ReactElementWrapper
  5. What's the point of this migration? (I mean is there any golden point that you think is good to be answered by providing this specific example?)

Question 2

Shallow render
Show how you can mock a troublesome component (like a 3rd party lib) that you want to treat as a passthrough/noop component in the test env

As far as I know, React Testing Library does not support shallow rendering and even it's not something that we should recommend people to use. But I think I'm misunderstood. Can you please clarify how would you do that?

Question 3

Triggering class methods in tests

Does this need any discussion? Like, do people use to write this kind of test in Enzyme? (I guess you mean how they use wrapper.instance())
This might be a general behavior, not just linked to Enzyme, and I don't know how widely it's used, and does it worth to talk about it? Since in the latest Enzyme version, they return null for stateless components when you can wrappper.instance() (I don't know what they used to return before!) , and more people these days are using stateless components. So this might not be a behavior. But you can correct me because I don't have much experience with Enzyme

Question 4

Working with Redux/Context

Should we care about Redux tests? I mean Redux is an implementation detail, and what we care about is that we do an action, and based on run the test against the component output. Should we care about the Redux store?
Again I guess I'm misunderstood :)

In the following day, I might ask some more questions to hopefully complete this page ASAP
Thanks a lot

@alexkrolick
Copy link
Collaborator

Question 1

Add a testid to the component => Change .find() to use the testid attribute (might now have issues with hostNodes/ReactElementWrappers both showing up) => migrate to RTL.

I don't understand about the example you want me to provide, can you please clarify?

This is what I understand:

  1. a component is needed
  2. A test that users wrapper.find should be used
  3. Now the test has to be changed to use testid (why?)
  4. I did not understand hostNodes/ReactElementWrapper
  5. What's the point of this migration? (I mean is there any golden point that you think is good to be answered by providing this specific example?)

If you have a pretty large test, you may want to rewrite part of the Enzyme test first. Otherwise you are changing both tests and implementation at the same time and that is risky.

The challenge with using test-ids in Enzyme is this:

let wrapper = mount(<Foo><Bar data-testid="abc" /></Foo>)
wrapper.find('[data-testid="abc"]').debug()
// ->
// Bar data-testid="abc"
//   div data-testid="abc"

Both the div (created by Bar) and the React component (Bar) have the testid attribute. It's not clear which one to use in the test and you might get more than one element when you only want one.

You can skip the Enzyme refactor if you trust that you can manage rewriting the test in RTL from scratch without missing anything. We could start documenting from this assumption I suppose.

Question 2

Shallow render
Show how you can mock a troublesome component (like a 3rd party lib) that you want to treat as a passthrough/noop component in the test env

As far as I know, React Testing Library does not support shallow rendering and even it's not something that we should recommend people to use. But I think I'm misunderstood. Can you please clarify how would you do that?

One reason to shallow render is it doesn't execute the render logic, which is good if a subcomponent has weird dependencies, side-effects, etc.

You can do something like this to create a fake component for a library:

jest.mock('google-maps-react', () => (props) => <google-map data-testid="google-map" {...props} />)

Question 3

Triggering class methods in tests

Does this need any discussion? Like, do people use to write this kind of test in Enzyme? (I guess you mean how they use wrapper.instance())
This might be a general behavior, not just linked to Enzyme, and I don't know how widely it's used, and does it worth to talk about it? Since in the latest Enzyme version, they return null for stateless components when you can wrappper.instance() (I don't know what they used to return before!) , and more people these days are using stateless components. So this might not be a behavior. But you can correct me because I don't have much experience with Enzyme

People access instance methods on class components and call them all the time. I don't agree that most people are using hooks in legacy codebases, which is where the most Enzyme migration is coming from.

Question 4

Working with Redux/Context

Should we care about Redux tests? I mean Redux is an implementation detail, and what we care about is that we do an action, and based on run the test against the component output. Should we care about the Redux store?

I'd cover the wrapper (?) options to mount and the equivalent option in RTL. Don't have to focus on Redux so much as Context in general (react-intl, styled-components, etc. all require a Provider).

@m98
Copy link
Member Author

m98 commented Aug 31, 2020

Dear @kentcdodds and @alexkrolick, I tried my best to add the requested changes.

In the current context, if there is anything that you think can be changed or improved, please let me know.

This PR is getting a little larger than expected!

Changes are welcome if you feel something is wrong both grammatically or technically.

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

I made a few modifications (and formatted with prettier). I'd love for someone to review/merge. Thanks so much @m98!

@kentcdodds kentcdodds merged commit 8cef60a into testing-library:master Sep 6, 2020
@kentcdodds
Copy link
Member

Thanks for your help @m98

@kentcdodds
Copy link
Member

@all-contributors please add @m98 for docs

@allcontributors
Copy link
Contributor

@kentcdodds

I've put up a pull request to add @m98! 🎉

@m98
Copy link
Member Author

m98 commented Sep 6, 2020

Thanks for accepting the PR @kentcdodds

I just browsed the Testing library website, but I could not see the "Migrate from Enzyme" page. It might be something that I've done wrongly!

Can you please check it out?

@kentcdodds
Copy link
Member

The page is on the site: https://testing-library.com/docs/react-testing-library/migrate

I'm not sure why it's not showing up in the sidebar. I don't know how that works actually 😅 Anyone wanna take a look at this?

@MatanBobi
Copy link
Member

MatanBobi commented Sep 8, 2020

I think that the name in the sidebar is wrong, it should be as the id of the page which is migrate and not migrate-from-enzyme 🙂

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

Successfully merging this pull request may close these issues.

Request to add more practical example to the document query page
5 participants