Add DatePicker tests using React Testing Library
#40754
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +183 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
noisysocks
added
[Type] Code Quality
mirka
reviewed
May 2, 2022
|
Thanks @mirka. I added a story comment and switched to |
kevin940726
reviewed
May 3, 2022
There was a problem hiding this comment.
Nice work @noisysocks 👍
✅ Storybook example looks good until we can migrate Knobs to Controls
✅ Unit tests pass and make sense to me
❓ I also had the same question regarding whether we could assert date selection based off of accessibility properties.
I'm not very familiar with react-dates and didn't find anything to solve that. Perhaps you'll have more ideas.
ciampo
reviewed
May 3, 2022
| ).toBe( true ); | ||
| } ); | ||
| it( 'should call onMonthPreviewed and onChange when a day in a different month is selected', async () => { | ||
| const user = userEvent.setup( { delay: null } ); |
There was a problem hiding this comment.
Would you mind making a few changes to these tests in order to align to how we use @testing-library/user-event in other components?
You'd need to rebase on top of trunk to include this version bump to user-event (don't forget to reinstall node_modules), and then switch to use jest fake timers + the advanceTimers option from userEvent
Example
diff --git a/packages/components/src/date-time/test/date.js b/packages/components/src/date-time/test/date.js
index f204dff80f..4cf66697f4 100644
--- a/packages/components/src/date-time/test/date.js
+++ b/packages/components/src/date-time/test/date.js
@@ -11,7 +11,20 @@ import 'react-dates/initialize';
*/
import DatePicker from '../date';
+const user = userEvent.setup( {
+ advanceTimers: jest.advanceTimersByTime,
+} );
+
describe( 'DatePicker', () => {
+ beforeEach( () => {
+ jest.useFakeTimers();
+ } );
+
+ afterEach( () => {
+ jest.runOnlyPendingTimers();
+ jest.useRealTimers();
+ } );
+
it( 'should highlight the current date', () => {
render( <DatePicker currentDate="2022-05-02T11:00:00" /> );
@@ -34,8 +47,6 @@ describe( 'DatePicker', () => {
} );
it( 'should call onChange when a day is selected', async () => {
- const user = userEvent.setup( { delay: null } );
-
const onChange = jest.fn();
render(
@@ -51,8 +62,6 @@ describe( 'DatePicker', () => {
} );
it( 'should call onMonthPreviewed and onChange when a day in a different month is selected', async () => {
- const user = userEvent.setup( { delay: null } );
-
const onMonthPreviewed = jest.fn();
const onChange = jest.fn();
@@ -99,8 +108,6 @@ describe( 'DatePicker', () => {
} );
it( 'should not allow invalid date to be selected', async () => {
- const user = userEvent.setup( { delay: null } );
-
const onChange = jest.fn();
render(I just merged a PR with similar changes today (#40790).
Also happy if you want to carry these changes in a follow-up !
There was a problem hiding this comment.
Opened a follow-up for this: #40800
Unfortunately the change causes a weird error in react-dates. Let's discuss in the other PR.
ciampo
reviewed
May 3, 2022
There was a problem hiding this comment.
Agree with the points made by @kevin940726 and @aaronrobertshaw .
Would it be also possible to add an entry to the CHANGELOG (this one could be under Internal)
noisysocks
force-pushed
the
add/more-DatePicker-tests
branch
from
7728df6 to
fce95e2
Compare
|
I've added a |
noisysocks
changed the title
DatePicker tests using React Testing Library
kevin940726
approved these changes
May 4, 2022
There was a problem hiding this comment.
LGTM!
Just a small nitpicking left here, but overall I think this is good enough 💯 .
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Rewrites the
DatePickertests to use React Testing Library and to test the component's basic functionality instead of simply thatreact-datesis called correctly.Why?
DatePickerworks. (I'm about to refactor it.)DatePicker's sister component,TimePicker.How?
Wrote new tests using React Testing Library. I focused on ensuring coverage for all of the public props.
I didn't touch
TimePicker's tests as they already look pretty good.I also added a new story that demonstrates the
isInvalidDateprop which was missing.Testing Instructions
npm run test