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] Fix flaky unit tests #16231

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cherniavskii
Copy link
Member

@cherniavskii cherniavskii commented Jan 17, 2025

Trying to fix https://app.circleci.com/pipelines/github/mui/mui-x/78497/workflows/58a06253-0d60-4507-8e42-a6ea37959cce/jobs/446898?invite=true#step-106-8172_107
I wasn't able to reproduce the failure locally (believe me, I tried), but I'm using the same approach as in #16219 that seemed to work.
Hopefully, this will reduce test fails on master and my inbox won't be flooded with false negatives 😅

@cherniavskii cherniavskii added test component: pickers This is the name of the generic UI component, not the React module! needs cherry-pick The PR should be cherry-picked to master after merge v7.x labels Jan 17, 2025
@mui-bot
Copy link

mui-bot commented Jan 17, 2025

Deploy preview: https://deploy-preview-16231--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 855a061

@cherniavskii cherniavskii marked this pull request as ready for review January 17, 2025 20:55
@cherniavskii cherniavskii requested a review from a team January 17, 2025 20:55
@lauri865
Copy link
Contributor

lauri865 commented Jan 18, 2025

I also tried to locally reproduce this and also other similar issues without any luck. I guess running it in a docker container with constrained resources could be a way to replicate Circle CI, but I don't have time to play around with it myself.

The downside to using waitFor is that it's more forgiving to any performance regressions / delayed state updates, as well as slows down tests (will wait for up to 1s by default). But until anyone finds a good way to reliably reproduce the errors locally, it's a pain to fix in any other way than to use waitFor, unfortunately.

It seems like theses tests started failing more consistently since recently (e.g. the locale test I encountered before, but usually cleared on the next run, but now seen it happen a couple of time in a row). Not sure why that is, but just an observation.

Probably turning off any animations/transitions in a testing environment could help though. Seems like it might be the root cause for both this and column pinning test, as the test itself doesn't seem to be failing, but other components get delayed state updates as a result of user interaction / re-rendering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! needs cherry-pick The PR should be cherry-picked to master after merge test v7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants