manager
manager copied to clipboard
test: Improve unit test stability when running locally
Description ๐
When running tests locally on develop, I'm getting anywhere between 3-8 failures each run. We additionally had a flaky unit test in CI during our last release which required a few re-runs to unblock the fire chief. This PR aims to address these failures and clean up a few warning messages.
Many of the failures are mitigated by increasing test timeouts -- I investigated the failures and the corresponding components and found that the tests and components were behaving exactly as expected, but when running alongside the rest of the test suite, the tests' performance drops dramatically (e.g. from 1-2s to 7-8s, or more). Going to post some more details as comments to this PR.
Changes ๐
- Upgrade Vitest from 1.2.x to 1.3.x.
- Increase test timeouts for flaky tests in
CreateCertificateDrawer.test.tsx,CreateAPITokenDrawer.test.tsx, andImageAndPassword.test.tsx - Remove calls to
act()tests to resolve some failures and warnings- In most cases, we were using React Testing Library functions that call
act()inside of our ownact()statements which triggered the issues (userEvent.*,findBy*, etc.)
- In most cases, we were using React Testing Library functions that call
Target release date ๐๏ธ
It'd be great to get this in by the next release just to reduce any risk of a unit test failure hindering the release process, but aside from that there's really no urgency here.
How to test ๐งช
This is tricky because most of these failures 1) only occur when running locally, 2) only occur when running the entire test suite (or some substantial portion of the test suite), and 3) seem to be performance related and therefore may vary from machine-to-machine.
The best way to confirm whether things are stable is to run the test suite locally:
yarn && yarn test
As an Author I have considered ๐ค
Check all that apply
- [ ] ๐ Doing a self review
- [ ] โ Our contribution guidelines
- [ ] ๐ค Splitting feature into small PRs
- [ ] โ Adding a changeset
- [ ] ๐งช Providing/Improving test coverage
- [ ] ๐ Removing all sensitive information from the code and PR description
- [ ] ๐ฉ Using a feature flag to protect the release
- [ ] ๐ฃ Providing comprehensive reproduction steps
- [ ] ๐ Providing or updating our documentation
- [ ] ๐ Scheduling a pair reviewing session
- [ ] ๐ฑ Providing mobile support
- [ ] โฟ Providing accessibility support
@hana-linode I added you as a reviewer because you're the only developer I'm aware of who has reproduced these failures so it would be really helpful if you could test these changes, but there's absolutely no rush/if you're busy and you don't get a chance that's totally fine too. Thanks!
Coverage Report: โ
Base Coverage: 82.3%
Current Coverage: 82.29%
I think I just got some flake on src/features/LoadBalancers/LoadBalancerCreate/LoadBalancerSummary/EditLoadBalancerConfigurations/EditRoutes/EditRouteDrawer.test.tsx. Maybe it needs some timeout ๐
https://github.com/linode/manager/actions/runs/8282874790/job/22664807267
I think I just got some flake on
src/features/LoadBalancers/LoadBalancerCreate/LoadBalancerSummary/EditLoadBalancerConfigurations/EditRoutes/EditRouteDrawer.test.tsx. Maybe it needs some timeout ๐https://github.com/linode/manager/actions/runs/8282874790/job/22664807267
@bnussman-akamai I have seen this one too! I spent a little time troubleshooting it, but couldn't reproduce it consistently enough to make any progress. Iirc its failure message indicated something other than a timeout, like an event not firing or something like that, but there wasn't a ton to go on!
If I have some time before this gets more approvals I'll try to take a second look, but if not I'll open a new ticket to investigate further so this doesn't slip through the cracks!
I'm still seeing some flaky tests but this is still an improvement ๐งน (Results screenshot)
Thanks @hana-linode, I'll check these out Monday morning -- the one test there timed out even though we increased the timeout to 10 seconds! Wonder if we need to increase it even more...
@jdamore-linode Awesome work! This should help alleviate some of the issues and improve developer experience. Looks like you've got some merge conflicts to resolve.