Example usage of isolate-components to replace enzyme shallow for testing hooks #43284
Conversation
| }); | ||
|
|
||
| it('Displays page buttons on each page', () => { |
There was a problem hiding this comment.
this was testing functionality of the Pagination component
There was a problem hiding this comment.
does isolate-component actually prevent you from testing the contents of the Pagination component? just checking my understanding that it is actually more like shallow than mount in that way.
There was a problem hiding this comment.
yes, it is more like shallow, it couldn't find the PaginationButton components in the tree
| @@ -166,8 +156,7 @@ describe('FormController', () => { | |||
| it('Does not submit when the last page has errors', () => { | |||
| setupErrored(); | |||
|
|
|||
| submitButton().simulate('submit'); | |||
| form.update(); | |||
There was a problem hiding this comment.
isolate-components seems to synchronously update the component as events happen, which makes it easy to get rendered data (no need to manually trigger an update or rerender)
|
@tim-dot-org the code changes look good to me. my only reservation is that this adds a new library for folks who write frontend code to learn. however, since it offers a material benefit over our existing use of |
I have the same reservations, tho this one does completely replace enzyme and has a simpler api. It is significantly less widely used though, and might have issues later on |
|
the main alternative approach I'm seeing is to use shallow rendering with https://www.npmjs.com/package/jest-react-hooks-shallow , which makes useEffect work as you would expect. any thoughts on which seems better @tim-dot-org ? the react team suggests using their own deep rendering solution and explicitly stubbing out child components, which seems worse. having a library to enforce that shallow rendering is happening seems like a big benefit for us. also curious if @madelynkasula has opinions. |
|
I think what we really want here is to merge this but have the understanding that we are still evaluating this option, like the tech radar idea that @agebhardt has been advocating for. right now we don't really have a process for evaluating new libraries -- once they are in the repo they are fair game to be copied indefinitely. |
I looked into that one too, but we're using mocha (instead of jest), potentially it just works with mocha, but i didn't wanna try and force that integration. It also seems to cause issues with existing |
|
One other thing to note! enzyme is planning on fixing |
|
good point about jest vs mocha. very interesting that enzyme is planning to use isolate-components! from that and reading the github issues, it does kind of seem that isolate-components may have some momentum behind it. I'm game to head in that direction! |
|
i feel pretty strongly that we need to move away from enzyme in the long term, likely toward react testing library (which isn't actually built/maintained by the react team), which has become the industry standard. airBnb transferred ownership of enzyme in early 2020, and the organization they transferred to is just 1 person. reading through the thread for this specific issue, it was opened in april 2019 and doesn't look like the maintainer will be able to address this anytime soon (which is understandable -- 1 person is holding this entire project on their back!). i worry that rather than enzyme getting updates, we will need to continue looking to outside libraries to get the testing functionality we need as react progresses. this is a useful article that i think encapsulates why folks have transitioned away from enzyme toward react testing library; there are good reasons to transition even outside of the concern around long-term maintenance. so, i don't think enzyme is the right direction in the long term, but that is obviously a big decision and a large amount of work. as tim mentioned, just getting react testing library set up may be a considerable task. (i don't actually have a good estimate, but i'm sure there are migration guides that could help us.) i plan to use react testing library in the component library; that's a new project, so the setup will be much easier, but should still give some insight into what it could look like in our main repo. this doesn't really answer your question, i don't think 😄 i'm not opposed to using this library, but i think we need to make a plan for the future of our frontend unit testing. maybe this is a topic for the architecture working group? @aaronwaggener and i have had some discussion about enzyme and its future already, so i'll chat with him about it. |
|
Thank you for those insights! That seems very convincing that we should move away from enzyme and I'd be on board with that. I need to read up a bit more on RTL to see how those tests are structured, because I haven't actually used it yet (it didn't seem to have any way to shallow render, but maybe that isn't as necessary as we currently think it is?). |
|
Thank you Maddie, that's great info! +1 for the goal of moving away from enzyme.
@tim-dot-org the React docs at https://reactjs.org/docs/testing.html#tools argue that deep rendering + mocking is one way to do shallow rendering tests:
I would much rather have a shallow rendering library, because then I don't have to remember to stub every child component in order to test my component in isolation. but if we had to use RTL for all our tests today then that's what we could convert our shallow rendering tests to do (which would be a whole lot of work). I'm not 100% sure this solution could be adapted from jest to mocha, so that could be worth investigating soon. |
|
If we were doing the work to switch off enzyme entirely to RTL, it might be worth looking into upgrading from mocha to jest at the same time. |
|
From Maddie's blog post link, I am also intrigued by this statement:
@madelynkasula perhaps the component library repo would be a good time to try out Jest, too! |
|
yes, i'm planning to use jest as well! i've used it in the past and i definitely agree with the quote you pulled from that blog post 😄 |
|
OK, so after today's eng meeting, it seems like React Testing Library would be a great candidate for a tech proposal! Until then, we should clarify what the current best practices are. I would propose the following best practices for our main repo for now, at least for when upgrading to functional components:
What do you think @madelynkasula and @tim-dot-org ? For now I mostly want to make as easy a path as possible for folks who want to upgrade existing components to use hooks. |
|
i like that proposal! we can use the component library as a testing ground for RTL/jest, and introducing isolate-components will allow us to better test hooks functionality right now, while we decide how we're moving forward in our main repo, which will take a long time to fully transition |
|
sounds great! I will merge this PR then |
|
great! I will make a note to maybe try out the |
Background: Enzyme's
shallowrenderer doesn't triggeruseEffecthooks in newer functional components, so you would normally need to usemountinstead which imposes some performance penalties and potentially has side effects that don't need to affect this component under test.isolate-components provides an alternative shallow renderer that does trigger
useEffect. It has a slightly different but equivalent api. This PR shows an example of refactoring a test that previously usedmount.Links
isolate-components docs
isolate-components npm
Enzyme issue
Testing story
Test passes!
Follow-up work
Communicate this alternative to team, progressively refactor mount tests and use in future tests against useEffect.