Should restoring mocks and other things happen in afterEach?

[julienw]

This is a follow up to #7654.

Currently restoring mocks happen in beforeEach, possibly because afterEach isn't run in case a test fails. I don't know if that's what users expect, at least the documentation doesn't say anything about that.

In #7654 we saw that it can impact tests when working with shared prototypes where some methods were mocked. Of course the main issue is the shared prototype thing, but by restoring mocks in beforeEach we wouldn't restore it for the final test in a test file, so this exacerbates the problem.

Relevant code (thanks @SimenB!):

https://github.com/facebook/jest/blob/faa52673f4d02ee1d273103ada9382a0805b54a4/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts#L56-L77
https://github.com/facebook/jest/blob/faa52673f4d02ee1d273103ada9382a0805b54a4/packages/jest-jasmine2/src/index.ts#L100-L120

My proposal here is:

1. Reset mocks in afterEach instead of, or possibly in addition to, beforeEach.
2. Run afterEach even if a test fails (working like a finally).

What do you all think?

[github-actions]

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]

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

[julienw]

My understanding is that this is still current. @SimenB

[mrazauskas]

beforeEach hook is better idea. This way all is predictable:

const someMock = jest.fn().mockImplementation(() => true);

beforeEach(() => {
  jest.restoreAllMocks();
});

test("one", () => {
  expect(someMock()).toBe(undefined); // mock was restored
});

test("two", () => {
  expect(someMock()).toBe(undefined); // mock was restored
});

Same with afterEach (add .only to the test "four" and it will fail, because the mock will not be restored):

const someMock = jest.fn().mockImplementation(() => true);

afterEach(() => {
  jest.restoreAllMocks();
});

test("three", () => {
  expect(someMock()).toBe(true); // mock will be restored after this test
});

test("four", () => {
  expect(someMock()).toBe(undefined); // mock was restored
});

---

Regarding restoring mocks after all tests. It seem like restoreAllMocks() is called after running each test file:

https://github.com/facebook/jest/blob/39a0e0588a9a06e087acba7434700290d45d8fc2/packages/jest-runtime/src/index.ts#L1341-L1343

[julienw]

@mrazauskas I don't mind doing it in beforeEach, but I believe it should be done in afterEach as well.
But see also #14080 for more issues around reseting mocks blindly.

[julienw]

beforeEach hook is better idea. This way all is predictable:

const someMock = jest.fn().mockImplementation(() => true);

beforeEach(() => {
  jest.restoreAllMocks();
});

test("one", () => {
  expect(someMock()).toBe(undefined); // mock was restored
});

test("two", () => {
  expect(someMock()).toBe(undefined); // mock was restored
});

Same with afterEach (add .only to the test "four" and it will fail, because the mock will not be restored):

const someMock = jest.fn().mockImplementation(() => true);

afterEach(() => {
  jest.restoreAllMocks();
});

test("three", () => {
  expect(someMock()).toBe(true); // mock will be restored after this test
});

test("four", () => {
  expect(someMock()).toBe(undefined); // mock was restored
});

This example makes no sense: why are you specifying a mock at first if you don't want to use it afterwards?

[julienw]

In the real life, we'll usually define a mock in both ways:

• either at the start of a file using jest.mock, and we want this to apply to the whole file
• or in a test (either in the body of the test or in beforeEach), and we want it to apply to this test only or this scope (for beforeEach)
• possibly both together: a default mock with jest.mock, but change it in some tests for specific beahviors (especially error behaviors)

First option without using jest.fn() works fine.
Third option is #14080.
Second option is this issue. I filed it a long time ago, so I need to think more to remember what the real-life problem is.

[mrazauskas]

This example makes no sense: why are you specifying a mock at first if you don't want to use it afterwards?

That was just a demo: what is the difference if jest.restoreAllMocks() is called afterEach instead of beforeEach test? jest.resetAllMocks() can be used instead. Would it make more sense?

[github-actions]

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.