-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[code-infra] Load tsx
files in visual regression
#19595
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
[code-infra] Load tsx
files in visual regression
#19595
Conversation
}); | ||
}); | ||
|
||
const pickersImports = import.meta.glob<React.ComponentType>('./pickers/**/*.{js,tsx}', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mui/explore I suppose the files under test/regressions/pickers
should be tested, but they weren't before, so I enabled it.
Let me know if you would like to keep this disabled or remove these files instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They were imported as is from the Core 3 years ago, never tested and just migrated when we did BCs
I'll let the @michelengelen and the customer success team decide what to do with them.
The tests are probably very outdated and not representative, but some might be valuable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michelengelen they don't seem to provide much right now as they are being clipped, I could fix them, but do we want to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO we can skip them, if they don't provide much value anyways. I haven't touched them ever, so I am mot really sure either. Maybe it's better to start a fresh set of tests? @flaviendelangle definitely has more insights into this than I do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's skip them then 👍
Deploy preview: https://deploy-preview-19595--material-ui-x.netlify.app/ Bundle size report
|
test/regressions/testsBySuite.ts
Outdated
}); | ||
Object.keys(chartsImports).forEach((path: string) => { | ||
const name = path.replace('./charts/', '').replace('.js', ''); | ||
const name = path.replace('./charts/', '').replace(/\.[A-z]+$/, ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: extract helper to avoid duplicate extension removing logic.
// string based variant is about 2.5x faster as well
const removeExtension = (filePath) => {
const extStart = filePath.lastIndexOf('.');
return extStart >= 0 ? filePath.slice(0, extStart) : filePath;
};
tsx
intest/regressions
docs/data/visual-regression-tests/
totest/regressions/charts/
pickers
files intest/regressions/pickers/