Bob/5817 return focus on archive close #5931
Conversation
| render(<TestContainer />); | ||
| expect(await screen.findByText(patients[0].lastName, { exact: false })); | ||
|
|
||
| const menu = (await screen.findAllByText("More actions"))[0]; |
There was a problem hiding this comment.
Can you wrap these actions in the act method or switch them to fireEvent.click? It is to not get the act warnings . You can add an .only to the it method to only run the new added coverage it.only( so you can test it isolated!
There was a problem hiding this comment.
So I did a little bit of digging around this because it was annoying me and I think I found that the root cause is a package versioning mismatch. Expo here.
TL;DR is that our versions of @testing-library/dom need to match throughout the tree in order to not generate unnecessary act warnings. Right now, running the yarn list on @testing-library/dom gives us the following:
├─ @testing-library/[email protected]
├─ @testing-library/[email protected]
│ └─ @testing-library/[email protected]
└─ [email protected]
└─ @testing-library/[email protected]
Once I updated the root installation of @testing-library/dom to 9.3.0 locally, the errors went away, even when using the userEvent action. For now, I just went ahead and replaced the one troublesome assertion with fireEvent so as to not add new errors, but hopefully the other errors will go away once we upgrade.
There was a problem hiding this comment.
ooo this is cool so if we upgrade @testing-library/dom to 9.3.0 the errors will get fixed in the other files as well?. I wonder why we haven't receive this upgrade from dependabot 🤔
There was a problem hiding this comment.
Yeah, when I updated locally the other related errors in the file also went away. I didn't run it on the whole test suite though, so unsure if it'll fix everything. I'm guessing that it's in the queue per my question here.
actually more digging reveals that this package is on a indirect dependency that we don't actually install, which is probably why there aren't any pull requests being made for it despite appearing in the scans. Upgrading the package manually unfortunately breaks some of the other unit tests. We also have some other dependencies that use this package as a subdependency that haven't upgraded yet, so we're somewhat constrained by those packages until then.
Upshot is that if we want to get rid of the warnings, we'll need to either manually install / keep in sync the subdependency or wait until the relevant packages upgrade. If we don't want to do that, we can either live with the warnings or write new tests using fireEvent where appropriate. Makes sense to me to do the later and just punt until the other packages get around to upgrading.
* only send common device values in fhir bundle * correct the service request to use test order loinc and completed status in bulk uploader fhir
* make uac all required * replace userEvent with fireEvent
* Migrated form to react-hook-forms * Updated unit test * fixed code smell * Fixed mocked answers for experian demo endpoint
fzhao99
requested review from
BobanL and
johanna-skylight
and removed request for
mehansen,
BobanL,
emyl3 and
johanna-skylight
|
Kudos, SonarCloud Quality Gate passed! |
FRONTEND PULL REQUEST
Related Issue
Fixes #5817
Changes Proposed
Testing
Screenshots / Demos
Screen.Recording.2023-06-05.at.1.03.17.PM.mov
Checklist for Author and Reviewer
Accessibility