[test] Remove createMount test util #42703
Conversation
aarongarciah
force-pushed
the
remove-create-mount
branch
from
1df7b07 to
e42bf4c
Compare
Netlify deploy previewhttps://deploy-preview-42703--material-ui.netlify.app/ Bundle size report |
| * returns a function that given the props for the styles object will return | ||
| * the css classes | ||
| * @param {object} styles argument for `makeStyles` | ||
| */ | ||
| function createGetClasses(styles) { | ||
| const useStyles = makeStyles(styles); | ||
| const output = {}; | ||
|
|
||
| function TestComponent(props) { | ||
| output.classes = useStyles(props); | ||
| return <div />; | ||
| } | ||
|
|
||
| return function mountWithProps(props) { | ||
| const wrapper = mount(<TestComponent {...props} />); | ||
| output.wrapper = wrapper; | ||
| return output; | ||
| }; | ||
| } |
There was a problem hiding this comment.
This custom setup has been replaced by using the renderHook directly. I think this makes tests easier to read by having less custom abstractions.
| 'MUI: The `styles` argument provided is invalid.\nYou are providing a function without a theme in the context.', | ||
| 'MUI: The `styles` argument provided is invalid.\nYou are providing a function without a theme in the context.', | ||
| 'Uncaught [TypeError: theme.spacing is not a function', |
There was a problem hiding this comment.
Not sure why errors are now duplicated, probably due to effects running twice now that renderHook is used. We pass strict: false to createRenderer (and to createMount before) in this test suite. Maybe we could look into creating our own renderHook wrapper with this option.
There was a problem hiding this comment.
This package is not compatible with strict mode, so yes, we need to have that option.
There was a problem hiding this comment.
We can tackle this in another PR to not make this PR even bigger.
| expect(sheetsRegistry.registry.length).to.equal(2); | ||
| expect(sheetsRegistry.toString()).to.equal(`.makeStyles-root-2 { | ||
| color: white; | ||
| background-color: black; | ||
| }`); | ||
|
|
||
| act(() => { |
There was a problem hiding this comment.
fireEvent already handles act for us.
aarongarciah
force-pushed
the
remove-create-mount
branch
from
bb99a64 to
4bbf1aa
Compare
aarongarciah
force-pushed
the
remove-create-mount
branch
from
4bbf1aa to
8583690
Compare
There was a problem hiding this comment.
Great job @aarongarciah! I only have one question: #42703 (comment)
Part of #42454
Warning
A lot of tests have been modified, pay special attention while reviewing. Highly recommended to review commit by commit.
Highlights:
createMountutil from@mui/internal-test-utilsin favor ofcreateRender.mountargument indescribeConformanceUnstyledcalls was unused, it has been removed from all Base UI tests.createRenderinstead ofcreateMount.findOutermostIntrinsicandwrapsIntrinsicElementfrom@mui/internal-test-utils. They seem unused across our code bases.Some of the tests have been updated to not rely on implementation details (e.g. props introspection) now that Enzyme is gone.