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

doc : jest fake timers : expect on setTimeout not working #11713

Open
christopheblin opened this issue Aug 2, 2021 · 20 comments
Open

doc : jest fake timers : expect on setTimeout not working #11713

christopheblin opened this issue Aug 2, 2021 · 20 comments

Comments

@christopheblin
Copy link

🐛 Bug Report

In https://jestjs.io/fr/docs/timer-mocks, we can see that we can assert that setTimeout has been called once : expect(setTimeout).toHaveBeenCalledTimes(1);

However, if you do this in a test, jest will complain :

expect(received).toHaveBeenCalledTimes(expected)

Matcher error: received value must be a mock or spy function

Received has type:  function
Received has value: [Function setTimeout]

  216 |     //TODO: test refresh works
> 217 |     expect(setTimeout).toHaveBeenCalledTimes(1);
      |                        ^
  218 |   });
  219 | });
  220 |

  at Object.<anonymous> (xxx.spec.ts:217:24)

I think the documentation should be fixed to explain how we can do ...

@Smrtnyk
Copy link

Smrtnyk commented Aug 3, 2021

if you are using jest 27, it uses modern timers now by default
you will need to spy on window.setTimeout beforeHands

@sigveio
Copy link
Contributor

sigveio commented Aug 3, 2021

I feel that the timer function used is an implementation detail, and that you would get more robust tests by instead looking at what you expect to happen once the task runs.

Instead of checking if setTimeout() has been called you could pass it a mocked function as the callback, fast forward in time with for example jest.runAllTicks(), and then assert that the mocked callback function was called with the parameters you expect. If you later replace setTimeout() with another timer implementation, it wouldn't necessarily break the test. Meaning you can have greater confidence in it.

And similarly, if you need to verify that callbacks are scheduled with a particular time or interval, it would make sense to use jest.advanceTimersByTime() and make assertions based on what you expect to happen at different points in time. Just checking if setTimeout() has been called with a given amount of milliseconds is generally not that meaningful, imo.

@hughrawlinson
Copy link

Since this issue is tagged with "needs repro", here is a repro.

I copied the example from the docs exactly, and setTimeout is not mocked. When I use legacy timers, the documented example works as expected. This suggests that the documentation demonstrates the legacy timers, not the modern timers. That document was last updated 8 months ago, and the commit history doesn't seem to suggest that the document was changed since the migration to modern timers.

I can't actually find a document on the jest site for modern timers. Hopefully this reflects my own inability to find the right search terms, rather than that jest has migrated to an undocumented timer mock API?

@sigveio
Copy link
Contributor

sigveio commented Aug 4, 2021

No, you are right; the current documentation is for the legacy timers and is outdated. I'm working on a new one 👍

@kleinfreund
Copy link

How is one supposed to solve this issue?

My tests start to fail as described in the inital report (i.e. I get a "received value must be a mock or spy function" error when invoking expect(setTimeout).not.toHaveBeenCalled() in a test).

Adding jest.spyOn(window, 'setTimeout') inexplicably produces a "ReferenceError: setTimeout is not defined" error:

node:internal/process/promises:246
          triggerUncaughtException(err, true /* fromPromise */);
          ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "ReferenceError: setTimeout is not defined".] {
  code: 'ERR_UNHANDLED_REJECTION'
}

I’m using testEnvironment: 'jsdom'. The function window.setTimeout does exist in the test, so I don’t really understand how it can appear as not defined to the test runner.

@sigveio
Copy link
Contributor

sigveio commented Aug 15, 2021

If you are using Jest 27 with its new default timer implementation, the current documentation is - as mentioned above - outdated. I have a draft for updated documentation in progress @ #11731. Perhaps the FAQ answer I added there could be of help?

I would try to think about why you are trying to assert against setTimeout, and if you could achieve the same (and perhaps even get more robust tests) with instead looking at what you expect to happen once the task scheduled by that setTimeout runs. For example designing your code in a way that allows you to pass in a spy as the callback for setTimeout and verify that this has been called the way you expect it to.

@kleinfreund
Copy link

kleinfreund commented Aug 15, 2021

Perhaps the FAQ answer I added there could be of help?

That does explain the situation very well, thank you.

I would try to think about why you are trying to assert against setTimeout, and if you could achieve the same (and perhaps even get more robust tests) with instead looking at what you expect to happen once the task scheduled by that setTimeout runs.

The specifics of my case make this undesirable (at least in my opinion). I’m updating a very small polling function that’s published as an npm package. Changing the code so that I’m able to pass a function as the setTimeout callback that I can set-up as a spy is not feasible (in my case, setTimeout is used in new Promise(resolve => setTimeout(resolve, delay))).

Now in truth, the assertions looking at setTimeout are always accompanied with assertions looking at the callback function that is passed to the poll function (and that I can spy on without problem). Timing-wise, they’re not however “next to each other”. Practically speaking, I could perhaps do without spying on window.setTimeout, but I would really prefer not to. Why wouldn’t I be able to spy on a global function? I understand how this could lead to testing internals of an implementation that might not contribute to a proper unit test, but that’s a decision a developer should be able to make rather than having the testing framework force this decision upon them. For now, I think I’m more comfortable relying on the legacy timer implementation.


Side note: Specifically what I’d like to still be able to do is assess whether certain calls happened in an expected order. I don’t much care about the exact processor time that elapses but rather the information that events A, B, and C happened before event D.

@sigveio
Copy link
Contributor

sigveio commented Aug 16, 2021

Why wouldn’t I be able to spy on a global function? I understand how this could lead to testing internals of an implementation that might not contribute to a proper unit test, but that’s a decision a developer should be able to make rather than having the testing framework force this decision upon them.

I went by all the reports about it not working and thought that perhaps it was sacrificed for the fact that relying on an external library greatly simplifies things for Jest.

But actually, I was partially wrong and should have tested it more thoroughly.

After you have enabled the fake timers you can spy on the global:

jest.spyOn(global, 'setTimeout');

That said; I do still stand by my comment on it most often being more favourable not to do so.

Side note: Specifically what I’d like to still be able to do is assess whether certain calls happened in an expected order. I don’t much care about the exact processor time that elapses but rather the information that events A, B, and C happened before event D.

When you use the modern fake timers, "processor time" should not play into the millisecond timing of when a given task can be expected to run though, because time is entirely faked. So with for example jest.advanceTimersByTime() you do have a lot of power.

I would also think that tasks under fake timers would run in the natural order they are scheduled in. So if you want to ignore the exact timing... and only care about the order... then perhaps you can use jest.runAllTimers() to fast forward in time and exhaust all the queues, and then toHaveBeenNthCalledWith() to verify them?

@sigveio
Copy link
Contributor

sigveio commented Aug 16, 2021

Oh, and @kleinfreund, I almost forgot; there's also jest.advanceTimersToNextTimer() that would allow you to step through the timers sequentially

@kleinfreund
Copy link

After you have enabled the fake timers you can spy on the global:

jest.spyOn(global, 'setTimeout');

Ah, interesting. I had tried both: jest.spyOn(window, 'setTimeout') and jest.spyOn(global, 'setTimeout'). Placing one such call at the start of the first test in my test suite led to the ReferenceError: setTimeout is not defined error. What I didn’t realize is that it actually works if I use a call to jest.spyOn(window, 'setTimeout') in all tests that assert whether the function has been called. So it turns out that spying on the setTimeout function works for both window or global as long as I register the spy in all tests making an assertion on it being called. I misread the ReferenceError: setTimeout is not defined as a principle issue with the attempt of registering the spy when it truth it’s likely caused by the missing spy in the other tests where I didn’t register it.

That said; I do still stand by my comment on it most often being more favourable not to do so.

I do agree.

@sigveio
Copy link
Contributor

sigveio commented Aug 17, 2021

🙌

global is more environment agnostic than window here - e.g. working in both node and jsdom

@kleinfreund
Copy link

kleinfreund commented Nov 2, 2021

I’m experiencing a very strange return of this issue in the same project as before.

I’ve made changes to my TypeScript source code (effectively adding 2 await statements to function calls) and doing so causes the jest to crash when running the tests:

 RUNS  src/poll.test.ts
node:internal/process/promises:246
          triggerUncaughtException(err, true /* fromPromise */);
          ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "ReferenceError: setTimeout is not defined".] {
  code: 'ERR_UNHANDLED_REJECTION'
}

The underlying error is once more “ReferenceError: setTimeout is not defined”. The tests don’t run at all. The test(…) blocks are completely unchanged and start off with the line jest.spyOn(global, 'setTimeout'). Removing it stops jest from crashing but—very much expectedly—causes my tests to fail. This happens on Jest 27 using fake timers and JSDOM as the test environment.

Reproduction steps:

  1. Clone https://github.com/kleinfreund/poll
  2. Run npm install
  3. Run npm test to see the tests pass
  4. Change src/poll.ts#L18 to if (await shouldStopPolling()) { (note the added await)
  5. Run npm test to see the tests crash

@ferrao
Copy link

ferrao commented Nov 22, 2021

I confirm that I also get ReferenceError: setTimeout is not defined in 27.0.3, the scenario is as follows:

afterEach(jest.useRealTimers);

it('test A', () => {
   jest.useFakeTimers();
   jest.spyOn(global, 'setTimeout');

   // run code which calls setTimeout
   expect(setTimeout).toHaveBeenCalledTimes(1);
   expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), 1000);
});

it('test B', () => {
   // run code which calls setTiemout

   // perform other assertions not related to setTimeout on same code as exercised in test A
  expect(...);
  expect(...);
}

Test A passes, but code executed by Test B fails, console.log(setTimeout) in that code returns undefined.
If I remove the spy on Test A, then Test B passes.

@sigveio , not testing setTimeout, but a callback instead as you mention in previous comments is not an option for me. My setTimeout performs a recursive call to the same function, which is not exposed. Something like:

module.exports = function() {

    const recursiveFunc = function() {

        const expire = ....;
         
        setTimeout(recursiveFunc, expire);

    }

}

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 17, 2023
@hughrawlinson
Copy link

Is the documentation still out of date? Would be a shame to let this issue be closed without updating the docs.

@github-actions github-actions bot removed the Stale label Mar 4, 2023
@ronkot
Copy link

ronkot commented Mar 9, 2023

I also encountered the ReferenceError: setTimeout is not defined error. I'm using [email protected] and the following setup code in one of the tests

    beforeAll(() => {
      jest.useFakeTimers();
      jest.spyOn(global, 'setTimeout');
    });

    afterAll(() => {
      jest.useRealTimers();
    });

causes my msw server closing in jest setup file

afterAll(() => server.close());

to fail with error ReferenceError: setTimeout is not defined.

I fixed this for now by just removing the server.close() step which seems to work, but is suboptimal.

Any ideas on how to debug this?

BTW, the line in msw source that throws the error is

export function nextTick(callback: () => void) {
  setTimeout(callback, 0)  // <-- THIS
}

export function nextTickAsync(callback: () => void) {
  return new Promise((resolve) => {
    setTimeout(() => {
      resolve(callback())
    }, 0)
  })
}

@ronkot
Copy link

ronkot commented Mar 13, 2023

OK, it seems that the error above was caused by msw - more specifically msw versions >= 0.42.0. I'll post an issue to msw repo.

@TrAndreiR
Copy link

What is the status of this issue? In my case, due to the timeouts being messed, I am having issues on the CI pipeline in github actions. Locally, everything looks fine, but in GA it seems that is not waiting enough for components to be rendered.

@JamesKyburz
Copy link

I just found at that using

jest.useFakeTimers()

doesn't work at all if the calls to timer functions are imported instead of using globalThis.

I had to remove

import { setInterval, clearInterval } from 'node:timers';

to get the tests to pass.

@loucadufault
Copy link

In my case ([email protected]), I found that it was the jest.useRealTimers() I was calling in the teardown of a prior test that was interfering with that test.

Repro:

describe('main', () => {
  describe('1', () => {
    beforeAll(() => {
      jest.useFakeTimers()
    })

    afterAll(() => {
      jest.useRealTimers()
    })

    it('does at least one test', () => {})
  })

  describe('2', () => {
    it('does another test', () => {
      unitUnderTest.myMethod()

      expect(setTimeout).toHaveBeenCalled()
      //     ^ ReferenceError: setTimeout is not defined
    })
  })
})

I found that simply commenting out the jest.useRealTimers() in the teardown fixed the 'does another test' test to have setTimeout defined. The better workaround I went with was to simply useFakeTimers/useRealTimers in the second describe block (even though I don't need them).

If I were to guess, something about useRealTimers is forgetting to restore the original setTimeout on the global object.

jest.config.ts

export default {
  automock: false,
  clearMocks: true,
  preset: 'ts-jest',
  rootDir: './',
  coverageDirectory: './coverage/frontend',
  coverageReporters: ['lcov', 'text'],
  testEnvironment: './test/__test__/JSDOMEnvironmentPatch.ts', // same as https://github.com/jsdom/jsdom/issues/1724#issuecomment-1446858041
  modulePathIgnorePatterns: ['<rootDir>/build'],
  prettierPath: require.resolve('prettier-2'),
} satisfies Config

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

Successfully merging a pull request may close this issue.