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

[test] Add missing async #706

Merged
merged 5 commits into from
Oct 8, 2024
Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Oct 6, 2024

Those async functions are noise, see the source of Act if you want to be sure: https://github.com/facebook/react/blob/main/packages/react/src/ReactAct.js.

Edit: Ok, no, this look required mui/mui-public#174

@mui-bot
Copy link

mui-bot commented Oct 6, 2024

Netlify deploy preview

https://deploy-preview-706--base-ui.netlify.app/

Generated by 🚫 dangerJS against b177558

@oliviertassinari oliviertassinari changed the title [test] Remove unused async [test] Add missing async Oct 6, 2024
@@ -153,7 +153,7 @@ describe('<Tabs.Root />', () => {
tabElements[index].id,
);

await act(async () => {
await act(() => {
Copy link
Member

@michaldudak michaldudak Oct 8, 2024

Choose a reason for hiding this comment

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

Fun fact: having an async here breaks 200+ other tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh strange, there is some black magic 😅

Copy link
Member

Choose a reason for hiding this comment

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

I'd love to switch to Vitest as soon as possible. At least test isolation works well there.

@michaldudak michaldudak merged commit 7b68042 into mui:master Oct 8, 2024
18 checks passed
@oliviertassinari oliviertassinari deleted the test-async branch October 8, 2024 10:04
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