waitFor with fake timers does not work with getByRole

[danny-does-stuff]

• @testing-library/dom version:
  7.22.6 (via @testing-library/react v10.4.9)
• Testing Framework and version:
  jest v26.6.0 (via react-scripts v4.0.3)
• DOM Environment:
  jsdom 16.6.0 (via react-scripts v4.0.3)

Relevant code or config:

import React, { useEffect, useState } from 'react'
import { render, screen, waitFor } from '@testing-library/react'

/**
 * Displays the passed `message` after `delay`
 */
function Test({ delay, message }) {
	console.log('rendering')
	const [delayedMessage, setDelayedMessage] = useState(null)
	useEffect(() => {
		setTimeout(() => {
			setDelayedMessage(message)
		}, delay)
	}, []) // Only run once

	return <h2>{delayedMessage}</h2>
}

describe('Fake Timers and waitFor and getByRole', () => {
	test('waitFor should work when used with fake timers and a long WAIT_TIME', async () => {
		jest.useFakeTimers()
		const WAIT_TIME = 80000
		const MESSAGE = 'Displayed Message'
		render(<Test delay={WAIT_TIME} message={MESSAGE} />)

		await waitFor(
			() => {
				expect(screen.getByRole('heading', { name: MESSAGE })).toBeTruthy()
				expect(screen.getByText(MESSAGE)).toBeTruthy()
			}
			// ,{ timeout: WAIT_TIME }
		)

		jest.useRealTimers()
	})
})

What you did:

Trying to test a component that depends on an interval to fetch some data. The code above is a much more simplified version with no network, no promises, etc.

What happened:

When trying to query the screen with getByRole, the test would fail. Using getByText allowed it to succeed. This only happens when the wait time is significantly larger than the default waitFor timeout.

Reproduction:

The code above is all that is needed to break it. Here's a minimal repro https://github.com/dannyharding10/waitFor-useFakeTimers-fail

Problem description:

This was extremely confusing to come across, especially after I saw #661 and the corresponding PR that fixed it (at least mostly 😅 ). I expected waitFor to either always work or always fail in this situation, but it seems to work only sometimes.

Suggested solution:

I don't think it makes sense to allow waitFor to just go forever, but it would be nice if the behavior were more consistent. If the asynchronous action took longer than the timeout for waitFor, then it should fail 100% of the time. Right now, it seems like it succeeds and fails conditionally depending on the complexity of the query (*ByRole vs *ByText), the duration (always fails with a crazy high delay, like 1_000_000+), etc.

[ahayes91]

@dannyharding10 When you use fake timers in Jest, you're removing the dependence on real time for the test, to allow your test environment to control the passage of time. https://jestjs.io/docs/timer-mocks#run-all-timers
So you need to explicitly tell Jest to fast-forward to the point where the timer has run - and because this updates the state of the component, you'll need to wrap that action that fast-forwards time in act to avoid a console warning.

This should work for you:

	test('waitFor should work when used with fake timers and a long WAIT_TIME', async () => {
		jest.useFakeTimers()
		const WAIT_TIME = 80000
		const MESSAGE = 'Displayed Message'
		render(<Test delay={WAIT_TIME} message={MESSAGE} />)
		act(() => {
                    jest.runAllTimers();
		});
		await waitFor(() => {
		   expect(screen.getByText(MESSAGE)).toBeInTheDocument()
		});
                expect(screen.getByRole('heading', { name: MESSAGE })).toBeInTheDocument()
		jest.useRealTimers()
	})

I'd also avoid putting byRole queries in a waitFor loop, as they are very computationally expensive - big thread about that here: #820.

Eslint would also throw a warning about your multiple assertions in a waitFor callback too, the following explains why:
https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/no-wait-for-multiple-assertions.md

[danny-does-stuff]

@ahayes91 according to pull request #662, we should not have to manually advance time, even when using fake timers. Maybe that PR was incomplete, leaving this issue?

[ianldgs]

I'm also experiencing fake timers not working with waitForElementToBeRemoved.
My async timeout is 5s and this times out in 400ms.

[danny-does-stuff]

Any update on this? I ran into another similar issue where waitForElementToBeRemoved does not seem to be advancing the fake time as it should according to #662

[eps1lon]

The code above is all that is needed to break it. Unfortunately jest.useFakeTimers() does not work in CodeSandbox.

@dannyharding10 Could you create a minimal, cloneable repository that reproduces the problem? This would help a lot debugging this issue.

[danny-does-stuff]

@eps1lon Here's a repro using the code from above. https://github.com/dannyharding10/waitFor-useFakeTimers-fail

Let me know if anything else is needed!

[hovsater]

@eps1lon do you need anything else to investigate this further? I see similar things when using jest.useFakeTimers(). For instance, the waitForElementToBeRemoved helper timeout instantly when fake timers are enabled.

[eps1lon]

The behavior in the repro is expected. The default interval for waitFor is 50ms i.e. every 50ms we check if the callback does not throw. Since you timeout after 800.000ms this means that we call the callback 16.000. The default timeout for a Jest test is 5.000ms. This means you expect getByRole to be dones within 0.315ms. But this is too little time since getByRole has some fairly expensive checks (see #820).

It's hard to give the correct advise here but fine tuning interval to better fit timeout seems like the best idea. Closing in favor of #820

[danny-does-stuff]

@eps1lon Thanks for looking into this! I'm assuming that you are getting the number 0.315ms from 5000 / 16000? If I understand correctly, you are saying that when we use fake timers, testing-library still uses the default interval of 50 ms, but that time is advanced virtually. So then the actual limit being imposed is not in waitFor, but rather on the entire test, which has a timeout of 5000 ms. I'm mostly thinking out loud, but I'm interested in knowing if I'm on the right track.

So is it just not advised to use getByRole inside of waitFor? Or maybe instead of using a large timeout in waitFor, it would be better to just use await act(() => advanceTimersByTime(largeInterval)) followed by the query.

[eps1lon]

I'm mostly thinking out loud, but I'm interested in knowing if I'm on the right track.

All correct 👍🏻

So is it just not advised to use getByRole inside of waitFor?

No that's perfectly fine. It just works with a consistent clock. Whether that is fake or real, the number of invocations of the callback are equal in real and fake clocks.