React testing library vs. enzyme

[julien51]

We currently use Enzyme for our testing of components. @cellog recommends using React Testing Library as a replacement.
It would be great to have people contribute to that issue with feedback on either approaches.
If we decide on moving, the earlier, the better!

[cellog]

I'll provide some context for the reason this project should use react-testing-library

enzyme works by hooking into the test renderer. On the surface, this seems fine, but there are some important hidden problems which will overwhelm the testing when the project grows.

1. enzyme has lagged well behind React in implementing new features, 9 months elapsed between the stable release of React 16.3 and support for the new context API, StrictMode, and many other features.
2. enzyme encourages testing the interface of things, rather than the user experience, which results in brittle tests

The second point is absolutely crucial. A great example is in the current test suite for unlock. Here is CreatorAccount.test.js:

import React from 'react'
import { shallow } from 'enzyme'
// Note, we use name import to import the non connected version of the component for testing
import { CreatorAccount } from '../../../components/creator/CreatorAccount'

describe('CreatorAccount', () => {
  it('should show the balance of the creator account', () => {
    const account = {
      address: '0x3ca206264762caf81a8f0a843bbb850987b41e16',
      balance: '17.73',
    }
    const network = {
      name: 4,
    }

    const wrapper = shallow(<CreatorAccount account={account} network={network} />)
    expect(wrapper.find('Balance').first().props().amount).toEqual('17.73')
  })
})

Note that the test first searches for a Balance component, then checks its React props to see if we are passing in the balance. This calcifies a couple of implementation details we should not fail on if implementation were to change:

1. the balance is displayed by a Balance component
2. its props must contain the balance

In fact, I ran into this exact problem when refactoring to add currency conversion. The test suddenly failed, and enzyme could not find the place where the balance was being used, no matter what I threw at it.

When switching to react-testing-library in this test, I instead changed the question from "Is the Balance component getting the correct balance in the props?" to "does the user see the correct balance?"

This key shift means we test the generated html to see if the balance is being properly displayed. In addition, when adding the currency conversion to dollars, this test would not fail, and can be extended to also check whether the converted currency amount is also being displayed to the user
Hence, the new test looks like:

import React from 'react'
import * as rtl from 'react-testing-library'
import 'jest-dom/extend-expect'
import { Provider } from 'react-redux'
// Note, we use name import to import the non connected version of the component for testing
import { CreatorAccount } from '../../../components/creator/CreatorAccount'
import createUnlockStore from '../../../createUnlockStore'

afterEach(rtl.cleanup)
describe('CreatorAccount', () => {
  it('should show the balance of the creator account', () => {
    const account = {
      address: '0x3ca206264762caf81a8f0a843bbb850987b41e16',
      balance: '200000000000000000',
    }
    const network = {
      name: 4,
    }
    const currency = {
      USD: 195.99,
    }

    const store = createUnlockStore({ currency })

    const wrapper = rtl.render(<Provider store={store}>
      <CreatorAccount account={account} network={network} />
    </Provider>)
    // eth value
    expect(wrapper.queryByText('0.2')).not.toBeNull()
    // usd value
    expect(wrapper.queryByText('39.20')).not.toBeNull()
  })
})

There's a few more lines of setup with rtl:

import * as rtl from 'react-testing-library'
import 'jest-dom/extend-expect'

instead of:

import { render } from 'enzyme'

but of course, hidden from view is the setting up of enzyme adapter for the React version, which must be updated every time React is upgraded. rtl uses react-dom's render, so it will work in perpetuity as long as react-dom render's API doesn't change.

The other new lines (adding in redux store) were necessitated in this test by changes to make Balance a connected component, and are not relevant to the discussion here. Here are the differences between the enzyme-based assertion and the rtl-based assertion:

enzyme:

    const wrapper = shallow(<CreatorAccount account={account} network={network} />)
    expect(wrapper.find('Balance').first().props().amount).toEqual('17.73')

react-testing-library:

    const wrapper = rtl.render(<Provider store={store}>
      <CreatorAccount account={account} network={network} />
    </Provider>)
    // eth value
    expect(wrapper.queryByText('0.2')).not.toBeNull()
    // usd value
    expect(wrapper.queryByText('39.20')).not.toBeNull()

I became interested in this problem because react-redux released a test version of 5.1.0 that broke against React 16.4, and it became clear that they were not testing against all the React versions they claimed to support. I wrote a test runner (source here: https://github.com/reduxjs/react-redux/tree/master/test) that runs the tests against all versions of React we support.

The first iteration involved converting the tests (which used React's test renderer) to using enzyme, because the API for the test renderer has changed drastically between 0.14 and 16.4 (the newest React version at the time). However, enzyme still did not support StrictMode at the time, so we had one hacky test to support it. Shortly after this conversion (which took about 2 hours of work, not including writing the test runner), I discovered rtl.

After only 1 hour of work, all the tests were converted to rtl, and it dramatically simplified the runner too. We no longer needed to install the custom versions of the test renderer and enzyme adapters for each React version, and it made adding new React versions as simple as adding the directory, copy/pasting package.json and updating the React/react-dom versions.

Some tests had to be reconceived, such as tests that confirmed react-redux was actually passing down context or running mapStateToProps, but this was easy to do. You can see the kinds of test utility constructs I made to make it easy to test props here:

https://github.com/reduxjs/react-redux/blob/master/test/components/connect.spec.js#L14

and a test that uses them:

https://github.com/reduxjs/react-redux/blob/master/test/components/connect.spec.js#L94

This test confirms that the redux store's state is correctly passed down through context and then as props to the child component simply by printing out each prop as text. It's almost the same as enzyme's testing of props, except that we could choose to completely change how the props are passed, and to which component. These are irrelevant implementation details that should not require updating tests.

So, for example, when react-redux moves to a hook-based implementation, most tests will not require any changes to work, even though the way we do things internally will have changed dramatically.

Hope this helps explain the reasoning behind the recommendation!

[julien51]

Ha! Thanks a lot @cellog. This was very thorough, clear and extremely helpful and probably wraps tis up: we need to rewrite the tests using rtl.

• migrate creatorAccount tests
• migrate Duration tests
• migrate showUnlockUserHasKeyToLock tests
• migrate OpenGraph tests
• migrate TwitterTags tests
• migrate Web3Provider tests
• migrate pages tests