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

"resetAllMocks" does not reset all mocks #7083

Closed
mushketyk opened this issue Oct 1, 2018 · 26 comments
Closed

"resetAllMocks" does not reset all mocks #7083

mushketyk opened this issue Oct 1, 2018 · 26 comments

Comments

@mushketyk
Copy link

mushketyk commented Oct 1, 2018

🐛 Bug Report

resetAllMocks does not reset mocks created with generateFromMetadata method

To Reproduce

Here is a utility method that I've created to create class mocks:

import * as jestMock from 'jest-mock'

export interface IClassType<T> {
  new (...args: any[]): T
}

export function mock<T>(classType: IClassType<T>): any {
  const mockType = jestMock.generateFromMetadata(jestMock.getMetadata(classType))
  return new mockType()
}

However when I use jest.resetAllMocks() mocks created by this function are not reset.

Here is a code example:

import { mock } from '../../../mock-utils'

import { Test } from './Test'

export class Test {
  public testMethod(param: number): boolean {
    return false
  }
}

describe('test mock', () => {
  const testMock = mock(Test)

  it('should reset mocks', () => {
    testMock.testMethod(123)
    testMock.testMethod.mockReset() // <--- works as expected
    expect(testMock.testMethod).not.toHaveBeenCalled() // <---- passes

    testMock.testMethod(123)
    jest.resetAllMocks() // <--- does not reset the mock
    expect(testMock.testMethod).not.toHaveBeenCalled() // <--- fails
  })
})

Expected behavior

The above test should pass.

Link to repl or repo (highly encouraged)

N/A

Run npx envinfo --preset jest

Paste the results here:

  System:
    OS: macOS High Sierra 10.13.1
    CPU: x64 Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
  Binaries:
    Node: 10.11.0 - /usr/local/bin/node
    Yarn: 1.10.1 - /usr/local/bin/yarn
    npm: 6.4.1 - ~/node_modules/.bin/npm
  npmPackages:
    @types/jest: ^23.1.0 => 23.3.2
    jest: ^22.0.0 => 22.4.4
@mushketyk
Copy link
Author

Updated to jest 23.6, but the issue is still there.

Could you look into this?

@rickhanlonii
Copy link
Member

@mushketyk looks like what you want to do with "reset" is actually "clear", so the bug is that mockReset is clearing the mock calls but resetAllMocks is not clearing the calls

If you change to mockClear and clearAllMocks does it work?

@mushketyk
Copy link
Author

@rickhanlonii I've tried to use clearAllMock instead of resetAllMocks but it still does not clear calls. So if I do in my tests:

  beforeEach(() => {
    jest.clearAllMocks()
  })

it does not help.

I even tried to use both clearAllMocks and resetAllMocks like this:

  beforeEach(() => {
    jest.resetAllMocks()
    jest.clearAllMocks()
  })

but this does not solve the issue as well.

The only thing that does help is resetting a particular mock, e.g. a single mock function on a mocked class like:

  beforeEach(() => {
    mockObject.method.resetMock()
  })

This clears calls from this mock

@maumercado
Copy link

I would like to take a stab at this as my " 👋 good first issue", any pointers or suggestions on fix/implementation?

@johannes-scharlach
Copy link

johannes-scharlach commented Nov 22, 2018

After playing with this topic for a bit, it seems like calling jestMock.clearAllMocks() will work on those mocks.

It seems like the file is required multiple times (within jest-runtime, jest-env-jsdom and jest-env-node) and the require cache is (probably on purpose) not always respected.

I'm not sure how to continue, possibly by attaching the mock state to global?

Apologies to @maumercado, I didn't mean to steal this from you, hope this info could help you solve it.

@rickhanlonii
Copy link
Member

@johannes-scharlach I'm not sure I follow - can you post a sample of what you tested?

@maumercado feel free to take a look as well!

@johannes-scharlach
Copy link

So what worked was the following

const jestMock = require('jest-mock');

describe('test mock', () => {
  function mock(classType) {
    const mockType = jestMock.generateFromMetadata(
      jestMock.getMetadata(classType),
    );
    return new mockType();
  }

  class Test {
    testMethod(param) {
      return false;
    }
  }
  const testMock = mock(Test);
  const otherMock = jest.fn();
  it('should reset mocks', () => {
    testMock.testMethod(123);
    testMock.testMethod.mockClear(); // <--- works as expected
    expect(testMock.testMethod).not.toHaveBeenCalled(); // <---- passes

    testMock.testMethod(123);
    jestMock.clearAllMocks(); // <--- does reset the mock
    expect(testMock.testMethod).not.toHaveBeenCalled(); // <--- passes
  });
});

So the this._mockState seems to be different between jest.clearAllMocks() and jestMock.clearAllMocks.

One possible solution here would be to use global._mockState instead of this._mockState, making it definitely the same.

The other thing I found out was that the constructor of the ModuleMockerClass is invoked 3 times when I run this for 1 test file: Once by jest-environment-node, by jest-environment-jsdom and by jest-runtime. (I found out about that by logging a stack trace in the constructor of ModuleMockerClass.). Maybe there is a better way to fix this, given that info?

@maumercado
Copy link

maumercado commented Nov 27, 2018

Quick update:

By @johannes-scharlach suggestion I have currently done the following change in the ModuleMockerClass:

constructor(global: Global) {
    if (global._moduleMocker) {
      return global._moduleMocker;
    }
    this._environmentGlobal = global;
    this._mockState = new WeakMap();
    this._mockConfigRegistry = new WeakMap();
    this._spyState = new Set();
    this.ModuleMocker = ModuleMockerClass;
    this._invocationCallCounter = 1;
    global._moduleMocker = this;
  }

with this change the use case specified here works, however when running yarn build && yarn test there are 27 failed tests, I'm currently looking at how did my change broke those tests.

Suggestions welcome!

@DaviWT
Copy link

DaviWT commented Dec 6, 2018

Hey! I'm following this issue for a college work and I'd like to help with anyway I can. I was able to reproduce the last solution from @maumercado , but I coudn't reach the "27 failed tests", I'm getting 74. How exactly are you testing? I'd need some help since it's my first time working with Jest.

@maumercado
Copy link

Hi @DaviWT, for testing I just do yarn build then yarn test, I am running node 10.13 maybe that's different for you.

Try running yarn build-clean then yarn build && yarn test see if anything changes.

@DaviWT
Copy link

DaviWT commented Dec 6, 2018

@maumercado I guess I don't have a script definition for yarn build in my package.json yet.

I'm not used to testing scripts, so any beginner advice is welcome, and I would appreciate it very much. For now I'm just trying to reproduce the bug and the possible solutions above in a proper way.

I have no initial intention to submit a solution officially, my goal is to learn as much as possible about Jest and open source development.

@maumercado
Copy link

@DaviWT no worries, any question is a good question.

So just to make this clear, you have forked the jest project locally and inside the jest project you are trying to run yarn build, but it is not inside your package.json?

@DaviWT
Copy link

DaviWT commented Dec 6, 2018

@maumercado Yes, exactly.

I'm able to execute yarn test because I have the following section in package.json :

"scripts": {
    "test": "jest"
}

I presume that there should be some specification for build as well inside the script section.

Another question, is the test only for the jest-mock package or for the whole Jest framework?

@maumercado
Copy link

@DaviWT The test is for the whole jest framework.. your Jest project package.json should look like this: https://github.com/facebook/jest/blob/master/package.json and you should be able to run the commands previously mentioned from the root of the jest project you just forked.

@DaviWT
Copy link

DaviWT commented Dec 6, 2018

@maumercado I see now, somehow my local directory was outdated from my own repository.

I'll do further testings as soon as possible. Thank you so much for the help!

@maumercado
Copy link

no problem! if you find anything worth discussing re: the issue at hand feel free to post!

@maumercado
Copy link

This is so far the tests failing for the module mocker only with the changes I did specified below:

FAIL  packages/jest-mock/src/__tests__/jest_mock.test.js
  ● moduleMocker › generateFromMetadata › does not mock methods from Object.prototype

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      at Object.toBe (packages/jest-mock/src/__tests__/jest_mock.test.js:231:9)

  ● moduleMocker › generateFromMetadata › does not mock methods from Function.prototype

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      at Object.toBe (packages/jest-mock/src/__tests__/jest_mock.test.js:269:69)

  ● moduleMocker › generateFromMetadata › does not mock methods from RegExp.prototype

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      at Object.toBe (packages/jest-mock/src/__tests__/jest_mock.test.js:300:69)

  ● moduleMocker › generateFromMetadata › mocks methods that are bound after mocking

    expect(function).toThrow(undefined)

    Received value must be a function, but instead "undefined" was found

      at Object.toThrow (packages/jest-mock/src/__tests__/jest_mock.test.js:342:27)

  ● moduleMocker › generateFromMetadata › supports mocking resolvable async functions

    expect(value).toBeInstanceOf(constructor)

    Expected constructor: Promise
    Received constructor: Promise
    Received value: {}

      at Object.toBeInstanceOf (packages/jest-mock/src/__tests__/jest_mock.test.js:530:23)

  ● moduleMocker › generateFromMetadata › supports mocking rejectable async functions

    expect(value).toBeInstanceOf(constructor)

    Expected constructor: Promise
    Received constructor: Promise
    Received value: {}

      at Object.toBeInstanceOf (packages/jest-mock/src/__tests__/jest_mock.test.js:553:23)

I am still not certain how to properly reconcile the global._mockstate when using jest-mock directly with the global._mockstate that is generated by the jest object, without breaking more tests.

At this point any pointers or help is greatly appreciated!

@phasnox
Copy link

phasnox commented Jul 12, 2019

Any news on this?

In my case mockfn.mockRestore() is not working

  it('should set and unset a doucmnet onKeyUp upon mounting ad unmounting process', () => {
    let addListener = jest.spyOn(window.document, 'addEventListener')
    let removeListener = jest.spyOn(window.document, 'removeEventListener')

    let wrapper = shallow(getWrapper())
    expect(addListener).not.toBeCalled()
    expect(removeListener).not.toBeCalled()

    wrapper = mount(getWrapper())
    expect(addListener).toBeCalled()
    expect(removeListener).not.toBeCalled()

    let handler = wrapper.instance().onKeyPressed
    let hasCall = ([event, func]) => event === 'keyup' && func === handler

    expect(addListener.mock.calls.some(hasCall)).toBe(true)

    wrapper.unmount()
    expect(removeListener.mock.calls.some(hasCall)).toBe(true)

    addListener.mockRestore()
    removeListener.mockRestore()
  })

  it('should not set any keyup when accessibility is false', () => {
    let addListener = jest.spyOn(window.document, 'addEventListener')

    mount(getWrapper({accessibility: false}))
    expect(addListener).not.toBeCalled()

    addListener.mockRestore()
  })

PS: I have also tried mockReset and mockClear

@caitecoll
Copy link

Is there an ETA on a fix for this or ideas for a workaround? I ran into this and it seems that a lot of others are as well based on the amount of +1s here: #7136

@finnigantime
Copy link

@caitecoll this workaround, mentioned on #7136, worked for me: #7136 (comment)

I think this ^ should be the default jest behavior. Leaking state between tests is an anti-pattern because it means test start to rely on running in a certain order (they rely on the side effects of previous tests). This is a problem because:

  1. A test may succeed when run in sequence but fail when run by itself (with -t).
  2. Tests cannot safely be moved around (order changed) without breaking.
  3. Tests cannot safely be split into batches and run in parallel.

IMO, clearing state between tests should be the default for these reasons and because the vast majority of projects do not require the performance benefits of not having to rebuild state before each test (and those projects that do can opt-into preserving state with config).

If it's very hard to change these defaults due to back-compat, then at least this deserves thorough documentation and a section on how to set up this config (rather than having to do an extensive grep through issues and stack overflow to find it).

@tomhardman0
Copy link

This is still a problem

@dgroh
Copy link

dgroh commented May 20, 2020

I'm having the same issue, at least very similar. I posted on StackOverflow. I'll be tracking this there and post here in case I find some solution.

https://stackoverflow.com/questions/61906896/spyon-working-with-jasmine-but-not-with-jest

@flozender
Copy link
Contributor

@SimenB I'd like to give this a try, until we can work something out for #10633

@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 14 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions
Copy link

github-actions bot commented May 1, 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 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.