CMS 5: Update jest tests to work with react 18 #1

[GuySartorelli]

Our javascript tests currently are incompatible with react 18 - both enzyme and react-dom/test-utils simply don't work with react 18, and no doubt there are other bits and pieces of test architecture we'll need to replace as well.

I tried replacing the above with the official react testing library but the fundamental philosophy of that library is incompatible with a large portion of our tests. The reality is we're going to have to rethink what we're testing and probably rewrite a considerable portion of it.

I have temporarily added a workaround to let the tests run using react 16 - but obviously that means if there's some failure that's specific to react 18 we're not going to catch it.

Acceptance criteria

• Whichever library we use for testing react component is documented and used consistently.
• All of our tests are updated to run with react 18
• The workaround to run tests with react 16 is removed

New issues created

• CMS 5: Update jest tests to work with react 18 #2 #1503
• CMS 5: Update jest tests to work with react 18 #3 #1506
• CMS 5: Update jest tests to work with react 18 #4 #1508
• CMS 5: Update jest tests to work with react 18 #5 #1509

PRs

• MNT Use React Testing Library silverstripe-cms#2849
• MNT Use React Testing Library silverstripe-elemental#1052
• MNT Use React Testing Library silverstripe-elemental-bannerblock#107
• MNT Use React Testing Library silverstripe-tagfield#240

[emteknetnz]

I've done a trial of converting campaign-admin to use react-testing-library.

First, a few clarifications about libraries:

• We still use jest as the base for js testing
• Enyzme + react-dom/test-utils do a similar thing, they build on top of jest to do react specific things such as rendering react. We use both but we probably should have only used one of them.
• Enyzme does not work with react 18 and never will as it has been abandoned
• react-dom/test-utils still seems to work with react-18 - but it's documentation is only on the legacy react docs site implying that it has a limited lifespan, and it also has a weird and limited API e.g. scryRenderedDOMComponentsWithClass(). The new react docs site is https://react.dev and it does have any documentation for react-dom/test-utils
• react-testing-library is a direct replacement for enzyme + react-dom/test-utils and seems to be the new "standard"

Note about react-testing-library queries:

• The broader testing-library has an opinionated philosophy about testing what the user sees on a screen, and avoid testing the internals of components. This includes API for querying that doesn't support css-selectors, instead encourages accessibility friendly queries
• Our existing tests use plenty of css-selectors
• Refactoring our existing tests to use a different selection method is out of scope, for this issue I simply want to update the react testing library we use with as little other refactoring as possible, because that will make the pull-requests far harder to peer-review.
• Also the refactoring to more use more accessibility friendly queries would need to happen as part of larger piece of work to make the CMS as a whole more accessibility friendly i.e. lots of tests cannot be refactored without the components themselves being updated first

Migration is still easy

• All of that said, you can simply just do this const el = render(<MyReactComponent {...props} >).container and you get a lovely DOMElement, so you can then just go const dececendantEl = el.querySelector('.some-class');. This is far simpler and nicer API then the old enzyme + react-dom/test-utils apis. This makes the migration far easier for use as we can use existing css-selector queries.
• For testing internal methods (which are all public) you can simply go const component = new MyReactComponent(props); and then let myResult = component.someMethod();. For testing internal state you simply go component.state. We're not actually use react when we do this. This feels like cheating but we are simply just unit-testing a javascript class for a lot of these unit tests.
• Uses these two methods I found the migration to react-testing-library very easy, at least for the campaign-admin trial

[emteknetnz]

Update on asset-admin

Different experience, not so good. Lots of tests in AssetDropzone-test.js are testing need to both render() the component in order to be the third party new DropzoneLib() to get rendered into the DOM, AND test the internals. We can't do both at the same time using react-testing-utils.

Looks we'll need to do additional work refactor a some of the tests so that they're testing the rendered output rather than internals

Some of these some of these tests are questionable, e.g. testing or directly modifying the internals of new DropzoneLib() which is a third party library, or for things like this we should probably just remove the tests

[emteknetnz]

I had a go at preserving the existing tests by only using the old ReactTestUtils which still functions, though it's on life support. It did work though it told me it red output in my terminal it would behave like react 17. IMO That's worse than our current solution of using react 16 without red warnings everywhere.

I did try to making a custom render() method life so that we could use in the jest test files.

const render = (jsx) => ReactDOM.createRoot(document.createElement('div')).render(jsx);

And while that renders, .render() no longers returns the rendered component in react 18, which is a change from react 17.

This is consistent with testing-library/react which also doesn't want to give you access to the rendered component.

All this is really pointing to the fact that we're going to need to refactor or just remove a decent chunk of the jest tests which require access to the rendered component.

[GuySartorelli]

PRs merged