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

Enable mui SelectField suite tests #1355

Merged
merged 3 commits into from
Jul 19, 2024
Merged

Conversation

Monteth
Copy link
Member

@Monteth Monteth commented Jun 21, 2024

Scope of this PR:

  • enable MUI SelectField suite tests
  • modify suite tests to pass in the MUI theme
    • mostly filter out non HTMLInputElements
    • skip if it doesn't pass and there is no clear way to fix it
  • remove local tests duplicated by the suites

The 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.

@github-actions github-actions bot added Area: Core Affects the uniforms package Area: Theme Affects some of the theme packages Theme: MUI Affects the uniforms-mui package labels Jun 21, 2024
Copy link

codecov bot commented Jun 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.23%. Comparing base (07869ec) to head (32e7bcb).

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.
📢 Have feedback on the report? Share it here.

@Monteth Monteth marked this pull request as ready for review June 21, 2024 11:28
const select = screen.getByRole('combobox');
expect(select).toBeDisabled();
});
skipTestIf(['mui', 'antd'].includes(options?.theme ?? ''))(
Copy link
Member

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 😅

Suggested change
skipTestIf(['mui', 'antd'].includes(options?.theme ?? ''))(
const shouldSkipTestBecauseOfTheme = ['mui', 'antd'].includes(options?.theme ?? '')
// somewhere above
// .
// .
// .
skipTestIf(shouldSkipTestBecauseOfTheme)(

Copy link
Member Author

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.

@Monteth Monteth merged commit fffdea5 into select-field-tests Jul 19, 2024
7 checks passed
@Monteth Monteth deleted the select-field-tests-mui branch July 19, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Core Affects the uniforms package Area: Theme Affects some of the theme packages Theme: MUI Affects the uniforms-mui package
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants