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] Use fake timers in visual regression tests #23464

Merged
merged 3 commits into from
Nov 10, 2020

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Nov 10, 2020

With #22692 we now have visual regression tests relying on Date which means they produce different output depending on the time they're run.

Using sinon#useFakeTimers to run the tests with the same (mocked) time. This also mocks all other timer related things so the tests should be even "more stable" though we haven't had any timer related flakyness. But it could still mean that we run less timer related stuff (e.g. passive effects?).

We'll see if this works. Otherwise I'll just stub the Date constructor.

@eps1lon eps1lon added the test label Nov 10, 2020
@mui-pr-bot
Copy link

mui-pr-bot commented Nov 10, 2020

No bundle size changes

Generated by 🚫 dangerJS against baa5613

@eps1lon
Copy link
Member Author

eps1lon commented Nov 10, 2020

Looks like it also improved other tests using Date (e.g. docs-getting-started-templates-sign-in/SignIn.png).

Though we now have some inputs in the error state.

Edit:

  • docs-components-date-time-picker/DateTimeValidation.png looks ok and having it in the invalid state actually increases coverage.

  • docs-components-date-time-picker/CustomDateTimePicker.png should not be invalid as far as I can tell.

@eps1lon eps1lon marked this pull request as ready for review November 10, 2020 19:16
@eps1lon
Copy link
Member Author

eps1lon commented Nov 10, 2020

Let's see if this'll work.

@eps1lon eps1lon merged commit 4fec9ee into mui:next Nov 10, 2020
@eps1lon eps1lon deleted the test/visual-regressions-time branch November 10, 2020 19:16
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