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

Migrated wrapField tests to @testing-library/react #1318

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

zaxoavoki
Copy link
Member

Is part of #1130

@github-actions github-actions bot added Area: Core Affects the uniforms package Area: Infra Affects the repository itself (e.g., CI, dependencies) Area: Theme Affects some of the theme packages Theme: AntD Affects the uniforms-antd package Theme: Bootstrap 3 Affects the uniforms-bootstrap3 package Theme: Bootstrap 4 Affects the uniforms-bootstrap4 package Theme: Bootstrap 5 Affects the uniforms-bootstrap5 package Theme: Material-UI Affects the uniforms-material package Theme: MUI Affects the uniforms-mui package Theme: Semantic UI Affects the uniforms-semantic package Theme: Unstyled Affects the uniforms-unstyled package labels Apr 19, 2024
@github-actions github-actions bot removed Theme: Semantic UI Affects the uniforms-semantic package Area: Infra Affects the repository itself (e.g., CI, dependencies) Theme: AntD Affects the uniforms-antd package Theme: Unstyled Affects the uniforms-unstyled package labels Apr 19, 2024
@zaxoavoki zaxoavoki self-assigned this Apr 19, 2024
@github-actions github-actions bot added the Theme: AntD Affects the uniforms-antd package label Apr 19, 2024
Copy link
Collaborator

@piotrpospiech piotrpospiech left a comment

Choose a reason for hiding this comment

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

This PR really points out the disadvantage of the suites approach (the same as SelectField suites, I guess)

  1. We talked we don't want to create flags for specific themes, but I see why it is needed here.
  2. Creating tests only for one theme in the suite or changing the expect check for the specific theme only makes this suite harder to read.

I think we need to consider keeping theme-specific tests in a separate file. We could keep core tests in the suites and keep theme specific in the theme package or create a separate file (we can have wrapField as a directory). The distinction can be necessary as we support different props, e.g., validateStatus in the AntD theme.

The same issue is true for the rest of the tests that are waiting to be moved to suites (mainly AntD and MUI).

packages/uniforms/__suites__/wrapField.tsx Outdated Show resolved Hide resolved
packages/uniforms/__suites__/wrapField.tsx Outdated Show resolved Hide resolved
packages/uniforms/__suites__/wrapField.tsx Outdated Show resolved Hide resolved
Copy link

codecov bot commented Apr 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.32%. Comparing base (4b3f109) to head (0b739ca).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1318      +/-   ##
==========================================
- Coverage   94.55%   94.32%   -0.24%     
==========================================
  Files         231      229       -2     
  Lines        3823     3754      -69     
  Branches     1030     1011      -19     
==========================================
- Hits         3615     3541      -74     
+ Misses         82       81       -1     
- Partials      126      132       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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: AntD Affects the uniforms-antd package Theme: Bootstrap 3 Affects the uniforms-bootstrap3 package Theme: Bootstrap 4 Affects the uniforms-bootstrap4 package Theme: Bootstrap 5 Affects the uniforms-bootstrap5 package Theme: Material-UI Affects the uniforms-material package Theme: MUI Affects the uniforms-mui package
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

None yet

3 participants