Migrated DateField tests to @testing-library/react
#1340
Conversation
github-actions
bot
added
Area: Theme
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1340 +/- ##
==========================================
- Coverage 95.05% 95.02% -0.04%
==========================================
Files 175 175
Lines 2892 2892
Branches 763 763
==========================================
- Hits 2749 2748 -1
Misses 59 59
- Partials 84 85 +1 ☔ View full report in Codecov by Sentry. |
zaxovaiko
requested review from
Monteth,
zendranm,
piotrpospiech,
ErnestTeluk and
a team
| import createContext from './_createContext'; | ||
| import mount from './_mount'; | ||
| const getClosestInput = (text: string) => | ||
| screen.getByText(text).closest('.ant-row')?.querySelector('input'); | ||
|
|
||
| test('<DateField> - renders an input', () => { |
There was a problem hiding this comment.
It doesn't look like an AntD-specific test. I checkout to 37fa8c1 commit (v3.10.1 tag). Multiple similar tests for DateField exist in the semantic theme. Did we remove them on purpose or by accident? Can you check other themes as well? Maybe those tests can be moved to suites.
There was a problem hiding this comment.
I switched to commit 37fa8c1 that you mentioned, and indeed, there were a lot more tests than we currently have. All themes have a few common tests that were only present in the AntD theme. I have moved them to suites, but I still needed to add the skipTestsForAntD flag due to some issues with how AntD handles dates.
# Conflicts: # packages/uniforms-material/__tests__/DateField.tsx
| export function testDateField( | ||
| DateField: ComponentType<any>, | ||
| { | ||
| skipTestAntD = true, |
There was a problem hiding this comment.
In the SelectField, you implemented theme?: 'antd';. It looks better.
| }); | ||
| const input = findClosestInputWithTestId('z'); | ||
| expect(input).toBeInTheDocument(); | ||
| expect(input!.value).toBe(getDateString(now, skipTestAntD)); |
There was a problem hiding this comment.
I get the idea, but it looks wrong. Even if you will use options.theme === 'antd' in the future, it won't be readable enough. Create a variable above:
const useISOFormat = options.theme === 'antd'
Maybe even put it above tests to avoid duplication (there are two places to refactor).
| }); | ||
| const input = findClosestInputWithTestId('z'); | ||
| expect(input).toBeInTheDocument(); | ||
| expect(input?.value).toBe(getDateString(now, skipTestAntD)); |
| async () => { | ||
| const date = '2022-02-02'; | ||
| const onChange = jest.fn(); | ||
| const extraProps = !skipTestAntD ? { showTime: false } : { type: 'date' }; |
There was a problem hiding this comment.
| const extraProps = !skipTestAntD ? { showTime: false } : { type: 'date' }; | |
| const extraProps = skipTestAntD ? { type: 'date' } : { showTime: false };``` |
| const findClosestInputWithTestId = (testId: string) => | ||
| screen.getByTestId(testId).closest('body')?.querySelector('input'); |
There was a problem hiding this comment.
It's actually very important to stick with the conventions of testing library.
There is a huge difference between find* and get*.
https://testing-library.com/docs/queries/about/#types-of-queries
| const findClosestInputWithTestId = (testId: string) => | |
| screen.getByTestId(testId).closest('body')?.querySelector('input'); | |
| const getClosestInputWithTestId = (testId: string) => | |
| screen.getByTestId(testId).closest('body')?.querySelector('input'); |
There was a problem hiding this comment.
Yeah, that makes sense! I've changed it.
Migrated tests for
DateFieldfor MUI and Material themes fromenzymeto@testing-library/react. Is part of #1130