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

MHP-2233: Bring in react-native-testing-library and add utility funct… #891

Closed
wants to merge 2 commits into from

Conversation

OzzieOrca
Copy link
Contributor

…ions

  • Update tests for App.js and route helpers

Enzyme made some progress towards supporting dive and the context API but then seems to have stalled...
enzymejs/enzyme#1647
enzymejs/enzyme#1960
enzymejs/enzyme#1966

I decided to just try this since enzyme has been holding up other package upgrades.
https://github.com/callstack/react-native-testing-library
https://callstack.github.io/react-native-testing-library/docs/getting-started
https://github.com/kentcdodds/react-testing-library (web version and inspiration)

Just looking for feedback at this point. I like that renderWithRedux helper I found. Doesn't have to be merged if we have hesitations. I haven't made any massive file changes yet. But we're using enzyme in a lot of places:

  • testSnapshotShallow - 311 references - I removed the return value and refactored the one test relying on it so this should just be changing to deep rendering (or some other strategy), updating snapshots, and fixing things that break from deep rendering. Deep rendering does seem to break a few things 😃
    Screen Shot 2019-03-29 at 10 35 17 PM
  • renderShallow - 400 references - This is where the real work will be. Could be a gradual migration but would hold back some package upgrades until it's done.
  • import ... from 'enzyme'; - 13 references (not counting the testUtils import)

https://kentcdodds.com/blog/why-i-never-use-shallow-rendering is a great article by the guy who wrote react-testing-library (which was the inspiration for react-native-testing-library). He doesn't like shallow rendering. Let me know if you agree. react-native-testing-library has a little bit of support for shallow rendering but it's mainly for snapshot generation. I like that shallow rendering makes for cleaner diffs but he makes good points about things not being well tested and figuring out when to dive through stuff has always been fun.

…ions

- Update tests for App.js and route helpers
@dsgoers
Copy link

dsgoers commented Apr 5, 2019

I read the article on why he doesn't use shallow rendering. I'm a firm believer in unit testing because it allows you to more easily test the code (and only that code) in each component. I think shallow rendering (and mocking, for that matter) makes testing easier since you don't need to depend at all on any other classes. To me, all I care about with a test is whether or not one specific component or function is executing the way it should.

In the MissionHub API, we create real objects for most of our tests, so even if you're testing a controller, all the callbacks on all the models execute and it makes testing really frustrating sometimes.

I understand the disadvantage that if the components call each other incorrectly, the tests will pass but the application will fail/behave incorrectly. However, idk if that's really going to help prevent things like icons getting cut off because when you render deeply the snapshot is huge

@reldredge71
Copy link
Contributor

I think unit testing works pretty well for us. However if integration testing proves to me more reliable for verifying our app functions properly, I would not be opposed to shifting to new test patterns. My major desire for our testbase is to have consistent structure, and maybe turning to something other than Enzyme if it is not being maintained well.

@OzzieOrca
Copy link
Contributor Author

Looks like Enzyme finally has a new release. I'll try it.

I guess I was drawn into that article cuz it presented a problem and a library that would keep you away from the problem. But that's the web version.

The react native version has both:

render – deeply renders given React element and returns helpers to query the output components.
shallow – shallowly renders given React component. It doesn't return any helpers to query the output.

The lack of helpers might be annoying. Seems like you can render deep, use the helpers, and then render shallow from the helpers. But I guess that forces you to make sure the whole component tree renders ok.

I'm just confused why both enzyme and react-test-render exist. enzyme was first published on December 6, 2015 and react-test-render on May 17, 2016. Which makes it sound like react-test-render was trying to solve a problem with 3rd party testing libraries.

If enzyme works going forward I guess it's easiest to just stick with it. I just hope it tracks React better in the future. You guys have any thoughts on new projects? I shared our frustrations with both Brian Z and the developers working some new missionhub-web stuff.

I'm also confused by what's encouraged. https://reactjs.org/docs/shallow-renderer.html says Shallow testing currently has some limitations, namely not supporting refs.

@OzzieOrca
Copy link
Contributor Author

Should I just close this for now? enzymejs/enzyme#2106 is still an outstanding issue and is preventing a react-redux upgrade past 5.x.

I'll steal that store test helper code sometime when I'm writing redux tests.

@reldredge71
Copy link
Contributor

@OzzieOrca do we still need this PR? Are we still waiting for Enzyme?

@OzzieOrca
Copy link
Contributor Author

I've been meaning to cherry pick the updated App.js test I worked on. I tried once and it was taking longer than I wanted to get it working. https://github.com/CruGlobal/missionhub-react-native/compare/MHP-2233-cleanup-app-test

I'll close this.

@OzzieOrca OzzieOrca closed this Jul 17, 2019
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.

None yet

3 participants