jest: [Bug]: jest.restoreAllMocks behavior changed/broken

@mrazauskas is there any way we can help those who will be struggling with this problem after us? Maybe an amendment to the CHANGELOG which links to migration guide and calls it out as an unexpectedly breaking change. Or a pinned issue which is a summary of the problem and a few options on what someone can do going forward, etc. I checked the npm download stats and there’s a significant number of users on versions prior to this change so even though it’s already been out for a month, there’s definitely millions of more people who will run into this issue.

@mrazauskas

What does bleed exactly? The state of a mock, i.e. count of calls etc; or you are not interested to have your mocks as mocks anymore?

The fact that it is mocked.

Let me explain it another way hopefully this will be clearer.

I see there to be two different kinds of mocks/spies. There’s test-specific ones and global ones.

• Global ones are ones like the matchMedia mock. They should be mocked all the time for every test in every file. You don’t ever want these to be reset, but you do want them to be cleared so the count of calls etc doesn’t carry over between test cases.
• Test-specific ones are created within a test block or a describe block. These are specific to the surrounding test code and therefore should be completely cleared (reset and restored to the original implementation) when the block they were declared within ends.

(A simple way to think of this is at the start of every describe/test block, it creates a snapshot of what mocks exist and restores to that snapshot when that block ends. Any additional mocks that were created get reverted, any mocks that were there in the snapshot persist.)

I know we could manually restore and clear each one individually, but this is very error prone and the whole reason the restoreMocks: true and clearMocks: true configs exist.

I had thought the previous behavior (maybe this was the bugged behavior) was closer to what I described above. Not sure if it wasn’t working exactly as I had expected, but it sure seemed like it. Now though after this has been “fixed”, I’m not sure how to accomplish what I described in a generic way. I either need to re-mock every global mock before every test, or make sure to restore every spy/mock that was created within a test before it ends.

That is why I suggested using clearMocks: true in configuration file (instead of restoreMocks: true) to clear the state automatically before each test.

So I’m guessing there’s no way to restore mocks that were created during a test before the next test? So if I spied on a method, changed the implementation to trigger some special behavior for that test case, now I need to make sure that I manually restore it before that test case ends? And somehow also restore it if the test case fails? That’s a big footgun. 😞

I totally understand that this may have been long-standing bugged behavior and I’m glad it’s fixed, but that might mean we need to add a new config option to accomplish what people were intending to do prior which is now very difficult to achieve.

Wow! I really appreciate you trying to explain this. It begs the question, with how complex this behavior was before (which existed for a long time) and many people depended on that functionality thinking it was intentional, why was this changed in a patch with no info? This is clearly very confusing behavior. It definitely is more clear now since it’s consistent, but also definitely breaking since it’s notably different than what it was doing for a long time.

My takeaway from all of this is that if you were using restoreMocks: true before and it was working for you, the change to make is to keep that setting, but now ensure that any mocks which are created outside of a test case are now inside of a beforeEach block so they are remocked before every test after the previous ones are cleared. This will ensure that each test runs with a clean slate like I believe many people were hoping for.

Example:

Before:

// fetchMock.js - imported in setup.js

global.fetch = jest.fn(() => {
    return {
        /* fetch response */
    };
});

After:

// fetchMock.js - imported in setup.js

beforeEach(() => {
    global.fetch = jest.fn(() => {
        return {
            /* fetch response */
        };
    });
});

If this is the case, which makes sense by reading the documentation, I think restoreAllMocks has been broken for a very long time, and a lot of our developers are using it incorrectly in their tests.

Maybe a breaking fix like this should be saved for the v30 release?

By setting the following dependencies my test will pass:

    "jest": "29.4.2",
    "jest-mock": "29.4.2",
    "jest-runtime": "29.4.2",
    "jest-runner": "29.4.2",
    "jest-worker": "29.4.2",
    "jest-each": "29.4.2",
    "jest-environment-node": "29.4.2",
    "@jest/environment": "29.4.2",
    "@jest/fake-timers": "29.4.2",
    "@jest/types": "29.4.2",
    "@types/node": "*",
    "jest-util": "29.4.2",
    "@jest/core": "29.4.2",
    "@jest/console": "29.4.2",
    "@jest/globals": "29.4.2",
    "@jest/reporters": "29.4.2",
    "@jest/schemas": "29.4.2",
    "@jest/source-map": "29.4.2",
    "@jest/test-result": "29.4.2",
    "@jest/transform": "29.4.2",
    "jest-changed-files": "29.4.2",
    "jest-config": "29.4.2",
    "jest-docblock": "29.4.2",
    "jest-haste-map": "29.4.2",
    "jest-leak-detector": "29.4.2",
    "jest-message-util": "29.4.2",
    "jest-regex-util": "29.4.2",
    "jest-resolve": "29.4.2",
    "jest-resolve-dependencies": "29.4.2",
    "jest-snapshot": "29.4.2",
    "jest-validate": "29.4.2",
    "jest-watcher": "29.4.2",
    "jest-cli": "29.4.2"

But the test @mrazauskas posted fails.

I’m not sure why the jest team uses the “^” in the version for the transitive jest dependenciest? It Makes it very hard to downgrade to a previous version.

If this leads to making more of our tests better, I’m all for it. We’ll be making the changes in our end even if it means in the thousands (we’ll find a way to automate them).

Because previously the restoreMocks almost did nothing, but now the restoreMocks seems equals to the reset to jest.fn() for those Non-SpyOn mocks + restore to the original implementation for those SpyOn mocks, so it cause a lot of test failed, since a lot of mock are defined on the first few lines of the file or the setupTest.js file, like some package mocks; Those have been restored;

so it just need remove restoreMocks: true if there is no special need to use it in the project

I agree, but it did not look breaking: #13867. In the other hand, I think swapping jest.restoreAllMocks() with jest.clearAllMocks() or restoreMocks: true with clearMocks: true should be enough. I don’t think users really wanted to restore their mocks before each test. If they really wanted that, an issue should be raised long time ago.

doing this fixed the majority of the test failures for us. The few that remained were all legit test issues that I’m surprised worked in earlier versions.

@ghiscoding Wait… I missed mockImplementationOnce. Hm… Those should work. Sorry. I will take a better look tomorrow. Thanks for pointing to this.

@ghiscoding Just checked your case. All you need is jest.clearAllMocks().

The test file has many assertions around exec, like this one: expect(exec).toHaveBeenCalled(). To make this work you have to clear count of calls and this is what clearAllMocks() will do.

jest.restoreAllMocks() does clear the count as well, but it also restores this mock (since 29.4.3 fixed a bug): https://github.com/lerna-lite/lerna-lite/blob/aa35a81736930755d71b3948cdf777da14defa86/packages/version/src/__tests__/git-push.spec.ts#L16

Currently only first tests will pass and all the others will have the mock restored (with implementation removed). This is what you see running this test file.

I found from older documentation it says “restoreAllMocks()” only affects spies, but now affects all:

jest.restoreAllMocks() and restoreMocks:[boolean]
Similar to resetAllMocks(), with one very important difference. It restores the original implementation of "spies". So, it goes like "replace mocks with jest.fn(), but replace spies with their original implementation".

So, in cases where we manually assign things with jest.fn() (not spies), we have to take care of implementation restoration ourselves as jest won't be doing it.

Yes, we fixed our test, after a day of trying to figure out what went wrong. Please don’t backport this fix to v27 or we will have a very big problem on our hands with all our old apps that just stop building…

The thing that makes this even worse is there is no way to “pin” a. specific version of jest because of the pointless use of carets “^” throughout all the nested dependencies: https://github.com/facebook/jest/issues/3391

Which makes trying to test against 29.4.2 almost impossible when 29.4.3 is released.

Maybe a breaking fix like this should be saved for the v30 release?

I agree, but it did not look breaking: #13867. In the other hand, I think swapping jest.restoreAllMocks() with jest.clearAllMocks() or restoreMocks: true with clearMocks: true should be enough. I don’t think users really wanted to restore their mocks before each test. If they really wanted that, an issue should be raised long time ago.