[test] Fix flaky tests #16257
[test] Fix flaky tests #16257
Conversation
|
Deploy preview: https://deploy-preview-16257--material-ui-x.netlify.app/ |
LukasTy
added
test
component: data grid
LukasTy
changed the title
| <LocalizationProvider dateAdapter={availableAdapters[adapterName]}> | ||
| <LocalizationProvider | ||
| dateAdapter={availableAdapters[adapterName]} | ||
| adapterLocale={adapterName === 'date-fns' ? enUS : 'en-US'} |
There was a problem hiding this comment.
Was it needed here, because of leaking config from the other test in the suite? 🤔
By default, we shouldn't need to explicitly set the enUS locale as it is the default if none is defined...
There was a problem hiding this comment.
Correct. Try running the test in isolation, it will fail 100% of the times with moment.js. It's specific to moment.js though, and due to importing side effects that seem to manipulate the locale of the global moment.js instance
import 'moment/locale/fr'; <- this as the last import seems to change the default locale
Maybe we could trigger locale change on top-level for momentjs adapter instead. Or if you know how to fix scoping for momentjs, that would be even better.
There was a problem hiding this comment.
It only seems to currently work in CI (and when you run the whole test suite) due to other tests changing the locale back to english. But I have seen the test fail in CI as well, I guess if this test happens to run before the other ones.
packages/x-date-pickers/src/DateCalendar/tests/localization.DateCalendar.test.tsx
Outdated
Show resolved
Hide resolved
| @@ -249,7 +249,7 @@ const PickersCalendarHeader = React.forwardRef(function PickersCalendarHeader( | |||
| </SwitchViewButton> | |||
| )} | |||
| </PickersCalendarHeaderLabelContainer> | |||
| <Fade in={view === 'day'}> | |||
| <Fade in={view === 'day'} appear={!reduceAnimations} enter={!reduceAnimations}> | |||
There was a problem hiding this comment.
Nice find. 👍
But it looks like there is still a hide/show animation when reduceAnimations is specified. 🤔
Does it really remove timeouts?
Have you considered going a similar route as PickersFadeTransitionGroup, where it renders a React.Fragment wrapper when reduceAnimations is true? 🤔
There was a problem hiding this comment.
That'd probably the correct way, but I tried to go the easy route to prove the root cause. There's an exit prop as well that should remove the hide animations. Even better might be to put reduceAnimations in a context and have wrappers for all animation components.
There was a problem hiding this comment.
That'd probably the correct way, but I tried to go the easy route to prove the root cause.
Gotcha. 👌
There's an exit prop as well that should remove the hide animations.
I tried locally, but it does not seem to change anything, the animation is still there.
If we are going for pure "reduce animations", then there should be no animations when it is toggled, as this prop is mainly an "accessibility" setting.
Even better might be to put
reduceAnimationsin a context and have wrappers for all animation components.
Great point, now that we have this context, we should be able to move this setting there. 👍
…teCalendar.test.tsx Co-authored-by: Lukas Tyla <[email protected]> Signed-off-by: Lauri <[email protected]>
| @@ -0,0 +1,2 @@ | |||
| export const isJSDOM = | |||
There was a problem hiding this comment.
I would keep the try/catch here as it was in useGridVirtualScroller so it doesn't fail in server env
There was a problem hiding this comment.
Why would it fail in server environment? Besides window not being defined, which it checks for.
There was a problem hiding this comment.
I didn't notice there was a check for window alread 👍🏻
cherniavskii
added
needs cherry-pick
LukasTy
changed the title
|
Cherry-pick PRs will be created targeting branches: v7.x |
Signed-off-by: Lauri <[email protected]> Co-authored-by: Lukas Tyla <[email protected]>
|
Well, some tests on karma are still flaky. 🙈 |
Fixes flaky tests due to delayed side-effects and animations that run in different cycles in CI due to constrained environments.
Removes some dead code (according to the comments and my testing).
Ref discussion in #16231
Side note: if someone knows how to fix global instances of momentjs being reused for tests, then most tests would work in parallel, and that would speed up (at least local) testing significantly.
pnpm test:unittakes210spnpm test:unit --paralleltakes<25s