Skip to content

test: fix flaky tests for focus and location mutation #944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 2, 2021

Conversation

koba04
Copy link
Collaborator

@koba04 koba04 commented Feb 1, 2021

Fixes #940

I've inspected why the focus tests are flaky.
I've found that some tests didn't wait for a deduping interval, which means that a mutation in the next test might be deduped and failed.
To fix this, I've added await act(() => sleep(1)) before the next mutation. But the intention of the code is very unclear, so I've separated it as another function called waitForDedupingInterval.

In addition, many of the current tests depend on a real timer, which makes tests flaky, so I've refactored to avoid depending on a real timer as much possible as I can by using screen.findByText.

I've also noticed that a test in use-swr-local-mutation.test.tsx is flaky, so I've fixed it too.

I think there are more flaky tests in SWR, so I'll fix them so that tests are more reliable. #932 and #933 are PRs for it.

@koba04 koba04 changed the title Fix flaky focus test test: fix flaky tests for focus and location mutation Feb 1, 2021
@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 1, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8d0032f:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration

@koba04 koba04 force-pushed the fix-flaky-focus-test branch from 891e0b4 to cd0e71d Compare February 1, 2021 16:40
@koba04 koba04 marked this pull request as ready for review February 2, 2021 05:37
Copy link
Member

@shuding shuding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

@shuding
Copy link
Member

shuding commented Feb 2, 2021

It might be a nitpick, but I think the name waitForNextTick will be better than waitForDedupingInterval. Because it might not be caused by the deduping here, the state broadcasting is also asynchronous (check #735).

(But it’s just a suggestion and will not block the PR, so I’ll merge this one for now 👍. Thanks!)

@shuding shuding merged commit ef47355 into vercel:master Feb 2, 2021
@koba04 koba04 deleted the fix-flaky-focus-test branch February 3, 2021 10:34
@koba04
Copy link
Collaborator Author

koba04 commented Feb 3, 2021

Thank you! waitForNextTick makes sense for me, so I'll create a PR for that👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test suite of focusThrottleInterval fails very often
2 participants