Switch away from using enzyme.mount in tests (components/higher-order/with-filters/test/index.js) #7793
Conversation
This switches all tests in `components/higher-order/with-filters/test/index.js` from using enzyme.mount to `React.TestUtilities`. This is because `enzyme` does not fully support React 16.3+ (and movement to do so is really slow). This will fix issues with breakage due to the enzyme incompatibility as components receive React 16.3+ features (such as `forwardRef` usage in #7557).
nerrad
added
the
[Type] Automated Testing
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import withFilters from '..'; | ||
|
|
||
| const assertExpectedHtml = ( wrapper, containerTag, expectedHTML ) => { | ||
| const element = TestUtils.findRenderedDOMComponentWithTag( wrapper, containerTag ); |
There was a problem hiding this comment.
Technically speaking, we should be searching for a root component in here. Did you consider using https://reactjs.org/docs/test-utils.html#findrenderedcomponentwithtype instead?
There was a problem hiding this comment.
I'm not sure I follow why I want to search for the root component here. Can you elaborate?TestUtils.findRenderedComponentWithType returns a component instance, what I want here is the DOM Element matching the given tag (wrapper is the root component which the given element is derived from).
There was a problem hiding this comment.
nvm I think I grasp now the reason for your comment. It's to ensure the html output for the entire component matches the expectation.
| import { mount, shallow } from 'enzyme'; | ||
| import { shallow } from 'enzyme'; | ||
| import TestUtils from 'react-dom/test-utils'; | ||
| import ReactDOM from 'react-dom'; |
There was a problem hiding this comment.
I guess this shouldn't refer react-dom directly, but rather @wordpress/element. I'm not quite sure what is the best way to handle it.
There was a problem hiding this comment.
hmm sure, but then we'd be indirectly testing the export from @wordpress/element right? This is being used more as a test utility in this case?
There was a problem hiding this comment.
It’s fine as is for the time being. We need to create our own lib anyways 😃
| import { mount, shallow } from 'enzyme'; | ||
| import { shallow } from 'enzyme'; | ||
| import TestUtils from 'react-dom/test-utils'; | ||
| import ReactDOM from 'react-dom'; |
There was a problem hiding this comment.
It’s fine as is for the time being. We need to create our own lib anyways 😃
|
|
||
| expect( wrapper.html() ).toBe( '<div>My component</div>' ); | ||
| expect( shallowWrapper.html() ).toBe( '<div>My component</div>' ); | ||
| shallowWrapper.unmount(); |
There was a problem hiding this comment.
No need to do it, afterEach will handle it
| @@ -82,12 +111,15 @@ describe( 'withFilters', () => { | |||
| ); | |||
| const EnhancedComponent = withFilters( hookName )( SpiedComponent ); | |||
|
|
|||
| wrapper = mount( <EnhancedComponent /> ); | |||
| wrapper = TestUtils.renderIntoDocument( getTestComponent( EnhancedComponent ) ); | |||
There was a problem hiding this comment.
Can we use destructuring when importing TestUtils to avoid using the variable??
There was a problem hiding this comment.
TestUtils is the default export from react-dom/test-utils. So I'd have to destructure on a subsequent statement. If renderIntoDocument was available as a direct import I'd definitely do it but its not. For now, I think it's a bit useful knowing at a glance that renderIntoDocument is part of TestUtils?
There was a problem hiding this comment.
It’s quite surprising but good yo know 😃
gziolo
deleted the
update/replace-usage-of-enzyme.mount-with-React.TestUtils-with-filters.js
branch
Description
This switches all tests in
components/higher-order/with-filters/test/index.jsfrom using enzyme.mount toReact.TestUtilities. This is becauseenzymedoes not fully support React 16.3+ (and movement to do so is really slow). This will fix issues with breakage due to the enzyme incompatibility as components receive React 16.3+ features (such asforwardRefusage in #7557).Checklist: