waitFor errors with a weird error if a test using fake timers stops (because of an error for example) while it's waiting

[julienw]

• @testing-library/dom version: 7.26.3 (but I also reproduce in 7.27.0)
• Testing Framework and version: Jest 26.6.3
• DOM Environment: jsdom 16.4.0
• node 12.18

Relevant code or config:

See below.

What you did:

Simply using findByText in some promise-based code while using fake timers, working on a new test.

What happened:

This is the error I get:

  ●  Cannot log after tests are done. Did you forget to wait for something async in your test?
    Attempted to log "A function to advance timers was called but the timers API is not mocked with fake timers. Call `jest.useFakeTimers()` in this test or enable fake timers globally by setting `"timers": "fake"` in the configuration file. This warning is likely a result of a default configuration change in Jest 15.

    Release Blog Post: https://jestjs.io/blog/2016/09/01/jest-15.html
    Stack Trace:

          Error: 
          at FakeTimers._checkFakeTimers (node_modules/@jest/fake-timers/build/legacyFakeTimers.js:436:13)
          at FakeTimers.advanceTimersByTime (node_modules/@jest/fake-timers/build/legacyFakeTimers.js:263:10)
          at node_modules/@testing-library/dom/dist/wait-for.js:60:14".

      at BufferedConsole.warn (node_modules/@jest/console/build/BufferedConsole.js:242:10)
      at FakeTimers._checkFakeTimers (node_modules/@jest/fake-timers/build/legacyFakeTimers.js:426:28)
      at FakeTimers.advanceTimersByTime (node_modules/@jest/fake-timers/build/legacyFakeTimers.js:263:10)
      at node_modules/@testing-library/dom/dist/wait-for.js:60:14

One main issue is that the error is repeated indefinitely, I have to ctrl-c to stop the runner. It also obscures the real error happening before.

Reproduction:

This is the simplest code showing the problem:

  beforeEach(() => {
    jest.useFakeTimers();
  });
  afterEach(() => {
    jest.useRealTimers();
  });
  it('errors with a weird error', () => {
    // wait forever
    waitFor(() => {
      throw Error();
    });
  });

Obviously in this code I forgot to "await" waitFor, as a result the test stops before waitFor ends.

An example closer to the real life is this:

  beforeEach(() => {
    jest.useFakeTimers();
  });
  afterEach(() => {
    jest.useRealTimers();
  });
  it('errors with a weird error', async () => {
    Promise.resolve().then(() => {
      throw Error('oops, error');
    });

    // wait forever
    await waitFor(() => {
      throw Error();
    });
  });

The problem is that the test errors before waitFor finishes, fake timers are torn down automatically, but the code in waitFor still wants to use the fake timers.

I couldn't make it fail on codesandbox, I believe because setImmediate is aliased to setTimeout in browsers, but here is a repository where I reproduce locally: https://github.com/julienw/jest-dom-library-issue

git clone https://github.com/julienw/jest-dom-library-issue
cd jest-dom-library-issue
yarn
yarn test

Suggested solution:

I believe that the most important issue is that this never stops. The timeout should set finished to false... but I think that the timeout never runs because the loop is too "tight" (it's using setImmediate, maybe it's using a microtask, but couldn't find anything conclusive about that -- but probably it has precedence over setTimeout).

Note: I could live with one such log that would let me see easily the real error.

Other ideas:

1. waitFor should know that the test stopped and stop right away. Is it possible to get notified about that? Should we register an beforeEach and afterEach callbacks to get notified of when a test starts and stops?
2. we check if fake timers are enabled in https://github.com/testing-library/dom-testing-library/blob/master/src/wait-for.js#L54, and then use it all the time in https://github.com/testing-library/dom-testing-library/blob/master/src/wait-for.js#L68, maybe we should check it again at each loop and bail out if this changed? Would that be a perf problem?

But again I could live with these 2 problems if this wasn't looping forever.

[julienw]

I checked that we couldn't have an infinite loop because of the tight loop by simply adding more timers, either jest or testing-library stops at one point.

[eps1lon]

I'm not sure this is caused by testing library. I suspect that this can happen to any async test where we switch back to real timers in afterEach. Seems like jest is re-scheduling every dangling fake timer as a real timer which is not desired in our case.

Generally curious how async cleanup logic is supposed to be handled. From the current behavior I can only assume that it has to know how the "clock works" i.e. are timers automatically advanced or should the cleanup logic advance them?

[julienw]

I'm not sure this is caused by testing library. I suspect that this can happen to any async test where we switch back to real timers in afterEach. Seems like jest is re-scheduling every dangling fake timer as a real timer which is not desired in our case.

Well, this is not what I'm seeing here. Actually there are no fake setTimeout in this failing test... (of course there are some in my real test). waitFor uses a real setTimeout to manage its timeout, even in a fake timers environment (see the "helpers" file).

So I'm not sure about what you're saying about rescheduling dangling fake timers.

[eps1lon]

useFakeTimers affects everything timer related. This includes Promises (which means async-await is affect), ticks, animation frames etc.

[julienw]

I believe that's not true.

1. The "legacy" fake timers (default in jest v26) mock only setTimeout, setInterval, clearTimeout, clearInterval, nextTick, setImmediate and clearImmediate according to their own documentation. The modern timers (using sinon) seem to mock a bit more things (including rAF and nextTick), but still not Promises.
2. dom testing library uses this trick to internally use the "real" functions for its own usage:

   dom-testing-library/src/helpers.js

   Lines 61 to 72 in 9b688f8

   	function getTimeFunctions() {
   	// istanbul ignore next
   	return {
   	clearTimeoutFn: globalObj.clearTimeout,
   	setImmediateFn: globalObj.setImmediate || setImmediatePolyfill,
   	setTimeoutFn: globalObj.setTimeout,
   	}
   	}
   	const {clearTimeoutFn, setImmediateFn, setTimeoutFn} = runWithRealTimers(
   	getTimeFunctions,
   	)

Am I missing something?

[kentcdodds]

I think it's reasonable to add a check in our while loop to detect whether we're still using fake timers and if we're not throw a helpful error message. Can probably do the opposite as well (when going from no fake timers to fake timers). This could help avoid this problem. I'm working on this now.

[kentcdodds]

PR Open: #832

[julienw]

Thanks @kentcdodds !

BTW I was wondering if we could avoid the loop altogether, by replacing

dom-testing-library/src/wait-for.js

Line 68 in 9494fdc

jest.advanceTimersByTime(interval)

with

jest.advanceTimersByTime(timeout)

Probably a bad idea (this could run more app code than the current behavior), but I thought it's worth considering because this reduces the complexity of this code.

[eps1lon]

I believe that's not true.

Whoops, you're correct. This feature is still pending: jestjs/jest#6876

[github-actions]

🎉 This issue has been resolved in version 7.28.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀