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

[Bug]: Calling useFakeTimers does not reset state #12362

Closed
stephenh opened this issue Feb 10, 2022 · 5 comments · Fixed by #12572
Closed

[Bug]: Calling useFakeTimers does not reset state #12362

stephenh opened this issue Feb 10, 2022 · 5 comments · Fixed by #12572

Comments

@stephenh
Copy link

Version

27.4.7

Steps to reproduce

  1. Clone https://github.com/stephenh/repros
  2. Checkout the jest-fake-timers branch
  3. Run the test, it and fails
beforeEach(() => {
  console.log("Calling useFakeTimers");
  jest.useFakeTimers("modern")
});

describe("fake-timers", () => {
  it("test 1", () => {
    expect(jest.getTimerCount()).toEqual(0);
    setTimeout(() => {}, 0);
    expect(jest.getTimerCount()).toEqual(1);
  })

  it("test 2", () => {
    expect(jest.getTimerCount()).toEqual(0);
  })
})

Expected behavior

The jest.getTimerCount() in test 2 should be 0, instead it is 1.

Actual behavior

A timer leaked from test 1 to test 2, despite calling useFakeTimers, which the docs:

https://jestjs.io/docs/timer-mocks

"Additionally, you need to call jest.useFakeTimers() to reset internal counters before each test."

Explicitly suggest that useFakeTimers resets internal counters, but it does not.

Additional context

No response

Environment

npx envinfo --preset jest
Need to install the following packages:
  envinfo
Ok to proceed? (y) y

  System:
    OS: Linux 5.13 Ubuntu 21.10 21.10 (Impish Indri)
    CPU: (24) x64 12th Gen Intel(R) Core(TM) i9-12900K
  Binaries:
    Node: 16.4.0 - ~/.asdf/installs/nodejs/16.4.0/bin/node
    Yarn: 1.22.17 - ~/.asdf/shims/yarn
    npm: 8.2.0 - ~/.asdf/plugins/nodejs/shims/npm
  npmPackages:
    jest: ^27.5.1 => 27.5.1 
@F3n67u
Copy link
Contributor

F3n67u commented Feb 27, 2022

@stephenh Thanks for the report.

In the implementation, jest.useFakeTimers doesn't reset internal counters if we have created a fake timer which is the problem. see:

https://github.com/facebook/jest/blob/1125f2f4a2dad60e449d7751d70ff620fcaf3be1/packages/jest-fake-timers/src/modernFakeTimers.ts#L96-L110

But jest.useRealTimers will does internal counters, so If add a jest.useRealTimers line in beforeEach, test will pass:

$ git --no-pager diff
diff --git a/src/faker-timers.test.ts b/src/faker-timers.test.ts
index 3da723c..c6ebd3d 100644
--- a/src/faker-timers.test.ts
+++ b/src/faker-timers.test.ts
@@ -1,6 +1,7 @@
 
 beforeEach(() => {
   console.log("Calling useFakeTimers");
+  jest.useRealTimers()
   jest.useFakeTimers("modern")
 });
 
$ npm test src/faker-timers.test.ts

> repros@1.0.0 test
> jest "src/faker-timers.test.ts"

  console.log
    Calling useFakeTimers

      at Object.<anonymous> (src/faker-timers.test.ts:3:11)

  console.log
    Calling useFakeTimers

      at Object.<anonymous> (src/faker-timers.test.ts:3:11)

 PASS  src/faker-timers.test.ts
  fake-timers
    ✓ test 1 (31 ms)
    ✓ test 2 (2 ms)

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        0.3 s, estimated 1 s

I am not sure whether is document is wrong or the code implementation, @SimenB could you help?

@SimenB
Copy link
Member

SimenB commented Feb 27, 2022

The thought was that "if already faking, do nothing when called". I guess we could throw instead of silently doing nothing, and if what you wanted was a reset, you'd do jest.useRealTimers(); jest.useFakeTimers("modern") like suggested?

@F3n67u
Copy link
Contributor

F3n67u commented Mar 7, 2022

I guess we could throw instead of silently doing nothing

It will break every suite which calls jest.useFakeTimers more than once without calling jest.useRealTimers(). For example:

describe("fake-timers", () => {
  it("test 1", () => {
    jest.useFakeTimers();
    
    // do something, omit
    
    jest.runAllTimers();
    
    // do some assert
  })

  it("test 2", () => {
    jest.useFakeTimers();
    
    // do something, omit
    
    jest.runAllTimers();
    
    // do some assert  })
})

I've seen many cases in my company's repo which I think it's very reasonable and will not be bitten by timer mock bec the global operation attribute.

Maybe we could just fix our document? from:

Additionally, you need to call jest.useFakeTimers() to reset internal counters before each test.

to

Additionally, you need to call jest.useRealTimers() to reset internal counters before each test.

@SimenB
Copy link
Member

SimenB commented Apr 5, 2022

@github-actions
Copy link

github-actions bot commented May 6, 2022

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants