Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SpeedDialIcon] Fix test for react 16.12 #18379

Merged
merged 2 commits into from
Nov 15, 2019
Merged

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Nov 15, 2019

Started failing on the react-next pipeline (https://app.circleci.com/jobs/github/mui-org/material-ui/125891)

I'll remove the resolutions before merging. Just making sure the react-next build is green again. It is green with react@next: https://circleci.com/workflow-run/ecebfcea-4f90-417c-a871-815808526b5f

@eps1lon eps1lon added the test label Nov 15, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Nov 15, 2019

No bundle size changes comparing a54729e...9533904

Generated by 🚫 dangerJS against 9533904

@eps1lon eps1lon merged commit 805d358 into mui:master Nov 15, 2019
@eps1lon eps1lon deleted the test/react-next branch November 15, 2019 09:42
@oliviertassinari
Copy link
Member

Do you know why it fail? Could it be a regression on React side (that might be fixed by the time a new version is released)?

@eps1lon
Copy link
Member Author

eps1lon commented Nov 15, 2019

Since enzyme simulates react internals any internal change of react internals might cause a regression in enzyme. We have a history of having issues with React.memo and enzyme#find. It's one of the reasons we're switching from enzyme.

@oliviertassinari
Copy link
Member

Fair enough, so in the long term, it doesn't worth looking at what changed, it's better to use testing library 👍 .

@eps1lon
Copy link
Member Author

eps1lon commented Nov 15, 2019

Fair enough, so in the long term, it doesn't worth looking at what changed, it's better to use testing library .

Yeah. There was some change related to react-is in 16.12 and I do remember brian saying it was ok with enzyme. There was also a related devtools change so I'm pretty confident react had no public regression. Enzyme is a different story though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants