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

[Autocomplete] Better isolate test case #23704

Merged
merged 2 commits into from
Nov 27, 2020

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Nov 24, 2020

This opportunity was found in #23487. These two tests are interesting in what happens when the user press Enter and more specifically if the event bubble up to a potential parent <form> element to trigger the submit. By refactoring the test, we avoid early returns like: https://github.com/mui-org/material-ui/blob/dfc94e03d391384155cbdd1a0e3b0fe16e2e1042/packages/material-ui/src/useAutocomplete/useAutocomplete.js#L731

@oliviertassinari oliviertassinari added test component: autocomplete This is the name of the generic UI component, not the React module! labels Nov 24, 2020
@mui-pr-bot
Copy link

mui-pr-bot commented Nov 24, 2020

No bundle size changes

Generated by 🚫 dangerJS against f75f820

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit scary to rely on these tests since Enter isn't actually triggering submit when dispatched programmatically. Let's hope these don't hide issues.

Looks like you overloaded existing tests though. Not sure this change actually "isolates" things.

Copy link
Member Author

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enter isn't actually triggering submit when dispatched programmatically.

True. It seems to be good enough in our case. It doesn't seem that we need this more resilient approach: https://github.com/mui-org/material-ui/blob/062ea1f734f7bb112f972a7147e8e08d391d6e1b/packages/material-ui/src/Autocomplete/Autocomplete.test.js#L483-L505

@oliviertassinari oliviertassinari merged commit 5115f08 into mui:next Nov 27, 2020
@oliviertassinari oliviertassinari deleted the test-autocomplete branch November 27, 2020 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants