-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Enzyme react container testing examples & docs #3799
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
Conversation
Deploy preview for redux-docs ready! Built with commit eae4f2b |
This PR is an Enzyme example of how to do the patterns shown with RTL here in this PR: #3708 as well as showing some Enzyme specific testing strategies that can supplement the testing strategy RTL uses. |
…s on objective tradeoffs
<VisibleTodoList /> | ||
</Provider> | ||
) | ||
expect(wrapper.find('TodoList').prop('todos')).toEqual([]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kentcdodds While I'm well aware of the advantages & drawbacks of testing this way, suppose you wanted to write this test, how would you go about writing this sort of test in RTL? Would you not just end up re-implementing Enzyme's affordances in a round about way, rolling your own abstractions to polyfill what RTL intentionally doesn't implement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm aware of the advantage & drawbacks, I'm not looking to debate those tradeoffs. I'm saying, assuming someone wants to make this tradeoff (which I'm aware you disagree with), RTL has no affordance for it like Enzyme does, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wha I would do is query the output for the todos that are rendered. The user doesn't care about the "TodoList" component or the "todos" props.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that's what you'd would do, and I wholeheartedly agree that it's preferable! 99.99% of the time!
Imagine a click handler that updates state, and then there's a rube goldberg machine of cascading updates you don't want to spend time testing right now. It's ideal to extract a small pure function from the click handler, and unit test that pure function (vs not writing any tests at all if you don't have time), but in extracting the pure function you may break the component.
It can be ideal to have a unit test that asserts on implementation details, like setting the state, so you can then in turn extract the pure function, and then in turn replace the "bad" unit test with a "good" unit test of the pure function. Having the option to work incrementally can be helpful, and experienced users may be able to use this option for good instead of evil.
My point is, whether you agree people should write these tests or not, RTL doesn't have the option to use escape hatches (which can be desirable or undesirable depending on who you ask). I'm just trying to confirm that in this situation, you'd have to make a predetermined tradeoff in RTL whereas in Enzyme you'd have the option to change the tradeoffs.
I'm aware we disagree on the "sometimes its ok to cheat" part, I'm just trying to make sure I'm not off spouting factual inaccuracies about the pros & cons of the frameworks.
|
||
describe("<TodoList />", () => { | ||
it('renders an empty list', () => { | ||
const wrapper = shallow( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kentcdodds I believe RTL also does not have this feature to mock out all children (I'm aware Enzyme does more than exactly this). You'd have to manually maintain explicit lists of mocks as they change over time for each test that is written in this style, in RTL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to engage you in a debate on this before, and only got this URL in response in the past as well. I'm well aware of your position on shallow :) Objectively speaking though, to write a test in this style in RTL requires manually maintaining a list of children to mock, correct? It has no auto-mock children feature like Angular or Enzyme, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've given links to my blog posts to literally thousands of people, so sorry that I don't remember engaging in this debate with you. That said, since you already know my opinion on it I'm not sure why you were asking...
The fact is, I don't write tests "in this style" and I think they're more harmful than helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries :) Thanks for replying.
I wasn't asking you if you think devs should should write these tests in general, I was asking if RTL has affordances to do a particular thing, which is an objective question not a matter of opinion:
You'd have to manually maintain explicit lists of mocks [of children] as they change over time for each test that is written in this style, in RTL?
Sorry if that was not clear, or any my comments came off as antagonizing in any way. I don't intend for my abrasive communication style to get in the way of the point I'm trying to make here, so sorry if it came off that way :)
I think they're more harmful than helpful.
Generally speaking, I would agree with you & I would recommend people avoid "this style" (asserting on implementation details) as well. However, having no tests can be even more harmful than having less than ideal tests written tactfully, so asserting on implementation details can be the lesser of two evils if you're in that pickle. I really like this article https://codingitwrong.com/2018/12/03/why-you-should-sometimes-test-implementation-details.html and would be interested in your thoughts after giving it a good read, if you were so inclined.
My position that is people can make a mess of their test suites with any library (although its much easier to make a mess with Enzyme than RTL). But if there is a mess of hard to test code that is entrenched in your project, for whatever reason, Enzyme's escape hatches can help a more experienced dev come in & "fix things" & can be a life saver... I agree Enzyme can give devs a huge foot gun with no warning on the tin. But dangerous tools can be useful, too...
@markerikson - continued from the other thread:
My intent is not to cause friction in the conversation or anyone's PR, my intent is to ensure Enzyme users find Redux approachable, and to help fix misconceptions that Enzyme cannot be used to write good tests.
There are viable approaches that are possible with Enzyme that are not possible in RTL. If you want it to only be about Redux, I personally feel we should avoid mentioning RTL or Enzyme at all then. If its worth having a RTL example, I fail to see why its not worth also having an Enzyme example, which is arguably the testing library which is more widely used currently.
The changes I'm proposing here are far from anything you characterize here, on the contrary linking to & endorsing only RTL & completely shunning Enzyme makes the docs opinionated on these things, where as having both examples or neither keeps it unbiased & focused on Redux, in my opinion. This isn't about Enzyme vs RTL, its about keeping the docs focused on Redux & keeping them unbiased about all the other things you mentioned. |
Also #1462 calls for elaboration on the testing docs. It could help to have some clarification in what exactly is in/out of scope, so the docs can be improved, without overshooting & becoming a treatise as you characterize. |
Could we remove the changes to the |
Co-authored-by: Tim Dorr <[email protected]>
@timdorr thoughts on removing altogether vs bifurcating into an isolated example, somewhere else? I feel strongly there is value in providing complete working examples. Having the docs is nice, but having working code without having to copy snippets out of a prosaic article is of huge importance for approachability, IMO |
I would disagree based on the number of docs updates that are purely code fixes/changes. Many folks like to build the examples by following along the docs, rather than skipping to the very end. Regardless, we should leave example changes to another PR. I would like to get the docs changes included, but just by themselves. |
Ok I'll work on splitting the PR later. I do think examples are useful, though, and the tradeoffs I identified are test framework agnostic - one could isolate the Redux wrapped component(s) in React Testing Library just as well, and I do think its reasonable to have example tests for React+Redux testing inside of an example that already showcases React+Redux. Also nothing about having examples means people have to skip the docs & jump to the example. The fact that the docs had code snippets changed seems unrelated to the fact the examples were "incomplete" as far as showing all the testing tradeoffs. I'm arguing for both docs & examples :) Just because we have docs doesn't obviate the need for examples & vice versa, IMO :) |
As much as I appreciate the effort that was put into writing this section, at this point I don't see a need to include that info here. |
My WIP example of testing strategies I'd propose showcasing in the redux docs for how to test a react app w/ container & integration tests style verification, as an alternative to RTL.
RTL is great & should be recommended for users who are starting out, but there are strong merits to choosing Enzyme over RTL for some users / projects. A lot of the techniques Enzyme allows for are outlined in Micheal Feather's "working effectively with legacy code" book. The high isolation test patterns can also be useful for error localization & documentation purposes, especially in more complex code bases, regardless of legacy code. Having high isolation tests does not preclude you from also having integration tests, and despite what some will say the "testing pyramid" is still a very valid concept for certain types of apps that aren't just CRUD apps (which also happens to be where Redux provides the most value).
EXAMPLE: Sometimes it is useful to have escape hatches to "break the rules", sometimes it is useful to have bumpers at the bowling alley to "save" you from a gutter ball. Both libraries have their own strong points, and allow for various & overlapping testing strategies.
My "ask" would be to make the docs unbiased & maintain official guidance for using both. I'll take my PR to Enzyme or make a blog post if its not wanted here ;) but Enzyme is an incredibly useful tool that has affordances RTL has taken a stance on not adding.