-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
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
[Stepper] Always pass state props to connector #21370
[Stepper] Always pass state props to connector #21370
Conversation
…th nonLinear prop
…on/material-ui into Issue-21325-StepConnector
Details of bundle changes.Comparing: 51b6ea9...34d41f8 Details of page changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard to follow if this is a breaking change, bug or feature. If existing tests still pass, please don't change them. Prefer adding new ones. If this is a bug the new tests should be failing on master.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start on the implementation side, on the testing side, if you could take Sebastian feedback into account it would be great. Basically, it's a bug fix, we shouldn't have to change any tests. Now, I understand that we were using the enzyme shallow API, which likely break as we reorganize the React structure (but the DOM structure should stay identical). Maybe we could have two different pull requests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we are good, if we run all the tests with only the changes in the .test.js (ignoring the changes to the source), the migrated tests from shallow to mount still passes while the new regression test with testing library fails.
@eps1lon @oliviertassinari Thanks for the review! About test updates - I updated only those tests which stopped working after changes. Some of them were an easy fix - changing |
Hello! This is a fix for #21325
@oliviertassinari I followed your suggestion on fixing this issue. But I had to rewrite a couple of tests to make them green again.
Closes #21325