-
-
Notifications
You must be signed in to change notification settings - Fork 244
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
Enable mui SelectField suite tests #1355
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## select-field-tests #1355 +/- ##
======================================================
+ Coverage 94.14% 94.23% +0.09%
======================================================
Files 200 200
Lines 3314 3314
Branches 895 895
======================================================
+ Hits 3120 3123 +3
Misses 70 70
+ Partials 124 121 -3 ☔ View full report in Codecov by Sentry. |
const select = screen.getByRole('combobox'); | ||
expect(select).toBeDisabled(); | ||
}); | ||
skipTestIf(['mui', 'antd'].includes(options?.theme ?? ''))( |
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.
[very optional]
TBH, I'm not convinced by this suggestion either, but at the same time, I don't know how to approach this change.
If you come up with a better name than shouldSkipTestBecauseOfTheme
then go for it 😅
skipTestIf(['mui', 'antd'].includes(options?.theme ?? ''))( | |
const shouldSkipTestBecauseOfTheme = ['mui', 'antd'].includes(options?.theme ?? '') | |
// somewhere above | |
// . | |
// . | |
// . | |
skipTestIf(shouldSkipTestBecauseOfTheme)( |
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.
I'd instead use isTheme(['mui', 'antd'])
to ensure flexibility and ease of change.
Scope of this PR:
HTMLInputElement
sThe local tests that cover the same area as the suites with the new "filter" were not removed, as I'm not sure if it is the correct approach. I want to ask you for your opinion on that.