Replace Enzyme with React Testing Library #1078
Conversation
As a test run, this replaces Enzyme with React Testing Library in the `source-info` test suite. Seems to work pretty well, although this is a simple(ish) case. This uses react-testing-library v12, which is far from current, but is the latest that works with React 16 (our current version) or 17.
This covers some fairly complex use cases, including out use of the legacy context API.
|
Should be all done, but I should do a second pass and make sure no more cleanup is needed. |
| it('displays the correct environment for the deprecated staging URL', () => { | ||
| const banner = mount(<EnvironmentBanner | ||
| apiUrl="https://api-staging.monitoring.envirodatagov.org" | ||
| />); | ||
|
|
||
| expect(banner.children().exists()).toBe(true); | ||
| expect(banner.text()).toMatch(/\bstaging\b/); |
There was a problem hiding this comment.
This URL has been deprecated so long that I think it’s fine to stop testing for it.
| uuid: 'adc70e01-0723-4a28-a8e4-ca4b551ed2ae..0772dc83-7966-4881-8901-eceb051b9536', | ||
| uuid_from: 'adc70e01-0723-4a28-a8e4-ca4b551ed2ae', | ||
| uuid_to: '0772dc83-7966-4881-8901-eceb051b9536', | ||
| uuid: '70aa8b7b-d485-48dc-8295-7635ff04dc5b..ed4aed23-424e-4326-ac75-c3ad72797bad', | ||
| uuid_from: '70aa8b7b-d485-48dc-8295-7635ff04dc5b', | ||
| uuid_to: 'ed4aed23-424e-4326-ac75-c3ad72797bad', |
There was a problem hiding this comment.
The previous IDs were not in the actual mock data for pages/versions. This wasn’t an issue with shallow rendering in Enzyme, but RTL doesn’t really have shallow rendering, so this needs to match up with the other mock data in order for it to work now (you could mock all the sub-components to alleviate this problem, but I think that’s not usually a great solution).
| const mockChangeFrom = simplePage.versions.find(v => v.uuid === mockChange.uuid_from); | ||
| const mockChangeTo = simplePage.versions.find(v => v.uuid === mockChange.uuid_to); |
There was a problem hiding this comment.
Similar to the above — with no shallow rendering, we need actual Version records to render, not the objects we had before that were missing a lot of normally-required fields.
| const match = { params: { pageId: simplePage.uuid } }; | ||
| const match = { | ||
| params: { | ||
| pageId: simplePage.uuid, | ||
| change: `${simplePage.versions[1].uuid}..${simplePage.versions[0].uuid}`, | ||
| } | ||
| }; |
There was a problem hiding this comment.
With full rendering, <PageDetails> tries to redirect and update the URL when change info is missing. This wasn’t an issue with Enzyme’s shallow rendering, where it would never run the lifecycle events that led to the redirect.
We should add some real tests of those redirects, but for now it’s much more straightforward to set up the tests so they don’t cause redirects (especially since the shallow rendering wouldn’t have been testing that anyway).
|
|
||
| if (sameContent) { | ||
| return <div className={className} styleName={styleName}> | ||
| return <div className={className} styleName={styleName} role="alert"> |
There was a problem hiding this comment.
This arguably always should have had an alert role. Adding it here also makes test assertions a little easier in RTL.
| } | ||
| else if (this.state.diffData.change_count === 0) { | ||
| return <div className={className} styleName={styleName}> | ||
| return <div className={className} styleName={styleName} role="alert"> |
There was a problem hiding this comment.
This arguably always should have had an alert role. Adding it here also makes test assertions a little easier in RTL.
| return ( | ||
| <div className="loading"> | ||
| <object type="image/svg+xml" data={infinityLoaderPath} /> | ||
| <object type="image/svg+xml" data={infinityLoaderPath}>Loading…</object> |
There was a problem hiding this comment.
This both makes testing easier in RTL and gives us an accessible fallback to the spinner icon.
|
Happily, some of the changes here also make for better tests. RTL tries to push you towards Selenium/interaction-test-style assertions (look for text or buttons or ARIA-role objects in the DOM and interact with them, rather than looking at the React components), which are a better, user-centric way to think about things. There are a couple places where it’s awkward, though. |
This app is trapped on v16 of React (current is v19) because of some old tooling. In particular, we depend on Enzyme for testing, and Enzyme never shipped a working version for React v17. The new replacement is React Testing Library, which has pretty good community support and has survived for 3 major React releases, so I think it’s relatively safe. This upgrades to it.
Note this uses React Testing Library v12, which is nowhere near current, but is the latest that is compatible with React 16. Newer versions require React 18, so we’ll need to upgrade this along with React (if and when we do that).
This also adds jest-dom, which is just a nicer set of DOM-specific assertions for Jest.
I don’t know if this will actually free us up to upgrade React yet, though. There are other problematic cobwebs in here, like the old version of React Router we use and our CSS modules issues (see #581), which could possibly be blockers.