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

What is the correct way to test state dependant "connected" HOC's #1025

Closed
PavelPolyakov opened this issue Jul 8, 2017 · 23 comments
Closed
Labels

Comments

@PavelPolyakov
Copy link

PavelPolyakov commented Jul 8, 2017

There are several issues where people ask how to test connected components, as it's not possible to setState for them. The answer is - we need export pure and connected components separately.
(here and here, for example)

However, there is another situation, which I've reproduced in this repository:
https://github.com/PavelPolyakov/enzyme-HOC-setState

Imagine we have a HOC which needs to grab some information from the redux store and attach it to the WrappedComponent. In the examples case it attaches the pokemon info:
https://github.com/PavelPolyakov/enzyme-HOC-setState/blob/master/src/appHOC.js

At the same time, there is a state condition in the HOC code itself, let's say we start some procedure on componentWillMount and don't want to show anything till this procedure is done. The procedure is asynchronous.

It's not possible to do neither:

root.find('AppHOC').setState({});

nor

root.find('AppHOC').state();

The latest could be very useful, so it's possible to write a proper async wait for the state.

What is the correct way to test it? Let's say I want to mock the asynchronous procedure during the tests. However, I'm not able to setState on the component directly, because this is not a root component. From the other point - I can not export it as a pure component, as it depends on the WrappedComponent.

As a dirty solution, it is possible to put your expect inside the setTimeout, however, it means we wait for the max time:
https://github.com/PavelPolyakov/enzyme-HOC-setState/blob/master/src/__tests__/appHOC.test.js

@ljharb , what would be the suggested way to overcome this situation using enzyme ?

Regards,

@ljharb
Copy link
Member

ljharb commented Jul 9, 2017

You do not need to export the inner component at all, and should definitely never change your code just for testing.

All of your tests for the inner component should wrap the outer component, and use .dive(). That way, you're exercising the wrapper every time, but you're able to directly interact with the inner component.

@PavelPolyakov
Copy link
Author

PavelPolyakov commented Jul 9, 2017

hi @ljharb ,

Thanks for the quick answer.
I found you mentioning dive in the issues, however, if I understand correctly - dive can be used only with shallow rendering: https://github.com/airbnb/enzyme/blob/master/docs/api/ShallowWrapper/dive.md .

In my example I use mount.

In the documentation it is stated:

Full DOM rendering is ideal for use cases where you have components that may interact with DOM APIs, or may require the full lifecycle in order to fully test the component (i.e., componentDidMount etc.)

Indeed I heavy rely on the lifecycle hooks and store updates. I've tried to swap mount with shallow in the real world example and it doesn't work, as store changes which I do were not applied (component properties were not updated).

Here is the reproduction of what I mean (in the branch mount-vs-shallow):
https://github.com/PavelPolyakov/enzyme-HOC-setState/blob/mount-vs-shallow/src/__tests__/appHOC.test.js

shallow example doesn't work, while mount still works.

I also update the HOC inself, so it better corresponds my real world case:
https://github.com/PavelPolyakov/enzyme-HOC-setState/blob/mount-vs-shallow/src/appHOC.js

What would the flow of testing the full lifecycle components?

Regards,

@ljharb
Copy link
Member

ljharb commented Jul 9, 2017

I'm mobile atm so I can't give you a full example, but if you're using mount, you should only be passing props at the top of the tree. In general tho, you should prefer shallow to mount - if you need the experimental lifecycle option to shallow, that's fine too.

In other words, you should be able to get very close to 100% coverage of your codebase with shallow alone (testing everything at each level) - you can even explicitly .instance().componentDidMount() etc when needed, and unit-test individual instance methods - and fall back to mount only when you really need to truly test post-mounting logic (which should be very rare and minimal, ideally).

@ljharb ljharb added the question label Jul 9, 2017
@PavelPolyakov
Copy link
Author

@ljharb
thank you for your clarification, I'd try to migrate everything to shallow.
have a good weekend!

I'd appreciate in case you find spare time to quickly check my shallow/mount example tomorrow (or any other day).

Thank you for the library!

@dabit1
Copy link

dabit1 commented Feb 1, 2018

I finally found a very good solution to get a wrapper from a decorated component. For shallowed components you can use dive() but there is no similar method for mounted components. This is the solution I did:

const wrapper = mount(shallow(<MyComponent />).get(0))

Works like a charm :)

@dschinkel
Copy link

dschinkel commented Dec 5, 2018

@dabit1 I'd encourage you to figure out how to use shallow and dive() better instead of a strange work around such as mount(shallow(<MyComponent />).get(0)). It's convoluting the purpose of each method with the type of testing.

I'm totally in agreement after doing TDD myself with @ljharb especially for TDD, I only use shallow and it works fine. Devs need to understand the difference between "unit tests" and "integration tests" which has everything to do with using mount vs shallow which is why airbnb have those two types/scope of rendering. It's more about learning testing that anything...and practicing to understand what we're talking about.

Side rant: Do not listen to others who tell you shallow is "bad" either.

@dabit1
Copy link

dabit1 commented Dec 5, 2018

@dschinkel who did say that workaround was not for intrgration tests? :) Anyway, TDD allows integration tests and creating more unit tests than integration tests is just a recommendation. Also, TDD in react unfortunately requires to use mount more than I would like.

@dschinkel
Copy link

dschinkel commented Dec 5, 2018

@dabit1 you don't understand TDD. You don't need mount and should prefer shallow when you TDD.

TDD allows integration tests and creating more unit tests than integration tests is just a recommendation
Also, TDD in react unfortunately requires to use mount more than I would like

wrong, having smaller focused isolated tests with True TDD it's not a "recommendation", who told you that? Seems like you're talking about "test after"...that's not "TDD". And you can and should use shallow when you TDD. But you first need to understand TDD in order to have this conversation...so feel free to ping me on it here: slack.wedotdd.com

TDD relies on writing small, focused, isolated tests as first class citizen (fail test Before writing only just enough production code to make it pass) as you progress through completing a feature... not integration tests and focuses on the Test Pyramid having unit tests that show unit tests being more valuable than integration tests. Relying on integration tests would never work with TDD. I'm actually building a course on this now: https://twitter.com/DaveSchinkel/status/1062267649235791873

@dabit1
Copy link

dabit1 commented Dec 5, 2018

@dschinkel who the hell are you? Some kind of TDD prophet? Give me an argument instead of just saying "relies on". I do TDD in Reactjs and React Native and I know the problematic you can find out there. TDD in React is not the same than in other technologies and if you really want to specify every feature you will have to use mount in some moments.

PD: @dschinkel mate don't edit your comments cause you are making that my replies don't make sense haha. Such a perfectionist, worse than me. Anyway, I like your effort to avoid mount function from enzyme and I cannot say you are doing anything in the wrong way, it's just what I said in the next comments what I think.

@dschinkel
Copy link

dschinkel commented Dec 5, 2018

who the hell are you?

I must be a TDD prophet

Anyway, sorry to ruffle your feathers.

I've done it for years. I'm happy to discuss it. I haven't had to use mount when I TDD. When you say creating unit tests is a only recommendation for TDD, that was a red flag to me.

I see you know Carlos Ble ;)
http://www.wedotdd.com/interviews/companies/7

@dabit1
Copy link

dabit1 commented Dec 5, 2018

Unit tests should be the most you write. But I don't agree that if you create some small integration tests you are not doing TDD. Apart of that Enzyme and React are quite particular... Also if I have to mount a component when I'm testing instead of adding extra code to the component I'm testing I definitely prefer it. It's a balance between clean and testable code.

@dschinkel
Copy link

dschinkel commented Dec 5, 2018

Cool.

You must be referring to what I call "Component/Acceptance" tests (outside tests in BDD/outside-in TDD). I still use shallow for those but that's my approach. I've found a way to do that which works and sharing that in my course.

@dabit1
Copy link

dabit1 commented Dec 5, 2018

@dschinkel yes I know Carlos Ble. I read his book several times. I started in the world of testing because of that book.
I'll read the interview later ;)

@dabit1
Copy link

dabit1 commented Dec 5, 2018

About what you call "Component/Acceptance" I may agree. But they are just words. At the end TDD is creating a failure test, making it pass and refactoring. If you want to speak about details you will find different approaches from different people. However as I said before I prefer to make it as easier as possible because apart of doing testing in the right way you also look for another things like clean code or a good testing architecture. And a good architecture always should be easy to understand.

@dabit1
Copy link

dabit1 commented Dec 5, 2018

By the way I would be happy to see your approach @dschinkel

@dabit1
Copy link

dabit1 commented Dec 5, 2018

@dschinkel I subscribed on your course, I'm curious to know how you manage BDD and TDD with React. I have a test assigment from a company I applied on my github. You can take a look at README and the project itself to check how I manage BDD and TDD with React:
https://github.com/dabit1/globalmovies

@dschinkel
Copy link

dschinkel commented Dec 5, 2018

Oh hey sorry I was busy. Thanks for subscribing. I really want to get people to realize what a gem enzyme really is. A lot of devs are looking for stuff that makes it easier when really, more magic is not going to help, what will is more practice with testing and enzyme, especially shallow which is what I want my course to really bring out and show people how to do right and well.

I'm curious to know how you manage BDD and TDD with React

I'll be covering that in Episode 2. Right now I am getting Episode 1 out, which is an "Enzyme Primer"

Having the goal of leaving .NET and MS code, I was able to TDD on backend Node for 6 mo. I wanted back on the FE though. So I stopped everything and spent 1.5 years building WeDoTDD.com working night and day figuring out FE and TDD with React as well as first 6 mo of that doing interviews, etc. first. When I was actually coding WeDoTDD.com and doing these interviews, I was not working, not getting paid, it was rough but that's how I learned it initially and tweaked my style along the way, hit all the brick walls you can probably hit with React TDD and found ways to do it well and get past those issues.

I then applied that knowledge and taught developers on teams doing React on big systems such as at Fox, and other .coms and paired with a few devs and tried out my style and workflow with it and it really worked well I found out, not just on my own site.

I've since then worked at 8th Light previously TDD'ing more on React but with GraphQL and renderProps + HOCs which I'm not too fond of because they're hard to test when they're all nested in a mess.

I can't wait till I start using React hooks to get back to "simplicity" hopefully and thus making people realize a ton of nested HOCs isn't going to help you, breaking apart stuff will and keeping your react modules small.

So I've practiced TDD with React + Enzyme going on 4 solid years now every day (with mostly shallow()) on the Job. Done TDD for about 6 years.

Sure I can take a look at your repo!

@dabit1
Copy link

dabit1 commented Dec 5, 2018

@dschinkel Nice. I really want to check how you do it out.
Please try to reread your comments before submitting them, you are making me feel crazy since I had to reread your comment a few times hehe.

@dschinkel
Copy link

dschinkel commented Dec 5, 2018

I reviewed yours. I am not a fan of Dodd's way of doing things which is why I'm so hell bent on getting mine out and the TDD way. He tries to say enzyme is too complicated but I think he falls short from experience. So I don't use his react-redux-test that I see there in your repo, and in fact some of how he's showing how he tests action creators, I do the same but my tests are even simpler. I'm not a huge fan of mocking frameworks and mostly use custom mocks when I need to but not a huge mocker either because my code is simple and decoupled.

Cucumber is fine although I don't use it, I do my BDD in mocha or jest describe and test names.

you are making me feel crazy since I had to reread your comment a few times hehe

you mean I write too much :P

@dabit1
Copy link

dabit1 commented Dec 5, 2018

haha no, you don't write too much, you edit too much.

About the way I did TDD, let's discuss it once I've done your course.

About Cucumber, I really think it is important since it ensures creating a non ambiguous features. It's an efficient way to define the requirements in a clear way for technical and non technical stackholders. Apart of that, Cucumber provides some interesting tools and many tools integrate it.

@dschinkel
Copy link

About Cucumber, I really think it is important since it ensures creating a non ambiguous features. It's an efficient way to define the requirements in a clear way for technical and non technical stackholders

yea if you can get your stakeholders actually involved in writing those specs, more power to you.

@dabit1
Copy link

dabit1 commented Dec 5, 2018

yea if you can get your stakeholders actually involved in writing those specs, more power to you.

Even if you are doing a project for yourself and you are the ony one who is going to work on it it is really useful because of what Gherkin enforces to you.

@PierreTurnbull
Copy link

PierreTurnbull commented Dec 23, 2021

why is it so complicated to manipulate a HOC component?
I've got 3 layers on top of my component (material ui's withStyles, intl's injectIntl, and a custom withAuth wrapper), and dabit1's solution worked great for moving past one of these, but I can't figure out how to move past the 2 others...

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

No branches or pull requests

5 participants