Skip to content
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

test(useQuery): Ensure we await the rendered text we expect #338

Merged
merged 1 commit into from Apr 13, 2020

Conversation

kamranayub
Copy link
Contributor

@kamranayub kamranayub commented Apr 9, 2020

I noticed that CI was failing intermittently on a test and it was receiving an act() warning. I believe it's because we should be awaiting the find of the text to allow for async work to happen in useQuery.

To reproduce locally, run yarn test then a for all tests, then hit Enter twice to do two full test runs in sequence. The subsequent runs should fail reliably on this test.

This seemed to be introduced after my PR #332 perhaps due to fixing the GC issue, and a state update happening that isn't waited for during the test. Other tests do use act to do assertions as well, plus using find for components that do async work seems a lot more reliable.

Changes

  • Fix act warning that happens on subsequent test runs for test should be in "success" state by default in manual mode

@kamranayub kamranayub marked this pull request as ready for review April 9, 2020 15:19
waitForElement,
fireEvent,
} from '@testing-library/react'
import { render, act, waitForElement, fireEvent } from '@testing-library/react'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prettier did this for me

@@ -152,9 +147,9 @@ describe('useQuery', () => {
)
}

const rendered = render(<Page />)
const { findByText } = render(<Page />)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suspicion: due to GC happening on subsequent tests that was fixed in #332, this may have been passing as a fluke. The test before this ensures it's in a loading state by default, which is what the rendered output looks like for this test when it was failing.

@tannerlinsley tannerlinsley merged commit 1a18671 into TanStack:master Apr 13, 2020
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.

None yet

2 participants