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] Skip Safari and Firefox on broken tests #4994

Merged
merged 2 commits into from
May 25, 2022

Conversation

alexfauquette
Copy link
Member

From #4645 (comment)

Skip tests on Firefox and Safari because they fail without apparent reason

@mui-bot
Copy link

mui-bot commented May 24, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 271.2 420.9 375.8 367.48 54.277
Sort 100k rows ms 468.9 764.8 713.6 667.86 111.345
Select 100k rows ms 103 227.8 189.5 170.12 45.079
Deselect 100k rows ms 109.9 252.2 158.5 172.2 48.813

Generated by 🚫 dangerJS against 13b339c

@oliviertassinari oliviertassinari changed the title [tests] skip safari and Firefox on broken tests [test] Skip Safari and Firefox on broken tests May 24, 2022
Comment on lines 198 to 200
if (/safari/i.test(window.navigator.userAgent) || /firefox/i.test(window.navigator.userAgent)) {
this.skip();
}
Copy link
Member

Choose a reason for hiding this comment

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

For each test, could we leave a comment on what's missing to make it work? I believe we have done this systematically in the past.

For instance:

Suggested change
if (/safari/i.test(window.navigator.userAgent) || /firefox/i.test(window.navigator.userAgent)) {
this.skip();
}
// Only run in supported browsers
if (typeof Touch === 'undefined') {
this.skip();
}

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Sounds easier to read

Copy link
Member

Choose a reason for hiding this comment

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

And more reliable, if tomorrow the API is available, then the test starts running.

@alexfauquette alexfauquette merged commit 59169bc into mui:master May 25, 2022
@alexfauquette alexfauquette deleted the skip-tests branch May 25, 2022 10:35
alexfauquette added a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022
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