-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[WNMGDS-3182] Updates two @testing-library
dependencies
#3458
base: jryan/react-18-upgrade
Are you sure you want to change the base?
[WNMGDS-3182] Updates two @testing-library
dependencies
#3458
Conversation
…ary/react-hooks` dep
…@testing-library/react`
…` and uses jest's modern fakeTimers
…ecause v14 of `@testing-library/user-event` only passes along `key`
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.
Some questions, but super close!
@@ -6,8 +6,7 @@ const usePreact = Boolean(JSON.parse(process.env.PREACT ?? 'true')); | |||
const preactAliases = usePreact | |||
? { | |||
react: 'preact/compat', | |||
'react-dom/test-utils': 'preact/test-utils', |
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.
Wait, why did these go away?
@@ -121,11 +124,11 @@ describe('SingleInputDateField', function () { | |||
// https://github.com/testing-library/dom-testing-library/issues/774#issuecomment-702574312 | |||
// but it isn't working | |||
// | |||
// it('selecting a day calls onChange', () => { | |||
// it('selecting a day calls onChange', async () => { |
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.
This test doesn't work even with the new version updates?
@@ -24,6 +27,11 @@ function expectInvalid(textField: HTMLElement) { | |||
} | |||
|
|||
describe('DateInput', () => { | |||
afterEach(() => { |
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.
We need this clean up for anytime we use fakeTimers, right? I think there are two tests that use fake timers but don't do this clean up. Is that ok?
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.
Seems like its: ChoiceList, ds-choice-list and IdleTimeout. Do those tests not need the cleanup for some reason?
Summary
@testing-library/preact
,@testing-library/react
, and@testing-library/user-event
and removes@testing-library/preact-hooks
and@testing-library/react-hooks
.userEvent.click
is now a promise that needs to be awaited and the docs recommend invoking a single session that more closely replicates user behaviorHow to test
npm install
npm run test:unit
,npm run test:unit:wc
, andnpm run test:unit:preact
main
and another is a new issue with theuseClickOutsideHandler.test.tsx
test. I do not propose either should block this work but open to feedback.Checklist
[WNMGDS-####] Title
or [NO-TICKET] if this is unticketed work.Type
(only one) label for this PR, if it is a breaking change, label should only beType: Breaking
Impacts
, multiple can be selected.If this is a change to code:
npm run test:unit
andnpm run test:browser:all
were each successfulnpm run test:unit:update
) and browser-test snapshots (npm run test:browser:all:update
)