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

Enable clearMocks, resetMocks, and restoreMocks by default #18768

Open
JamieMagee opened this issue Nov 4, 2022 · 3 comments
Open

Enable clearMocks, resetMocks, and restoreMocks by default #18768

JamieMagee opened this issue Nov 4, 2022 · 3 comments
Labels
priority-4-low Low priority, unlikely to be done unless it becomes important to more people type:refactor Refactoring or improving of existing code

Comments

@JamieMagee
Copy link
Contributor

Describe the proposed change(s).

Jest config allows you to specify 3 flags regarding mocks in increasing order of strictness:

  1. clearMocks: true clears the call count and args of any mocks before each test.
  2. resetMocks: true goes a step further, and also clears any mock implementations / return values defined before each test.
  3. restoreMocks: true is the strictest and actually removes all mocks from all functions before each test.

The default, unfortunately, is not to do any of this, which means out-of-the-box Jest makes it very easy to write unit tests that unintentionally rely on test ordering.

I think the bare minimum should be to enable clearMocks in jest.config.ts (and remove any explicit jest.clearAllMocks() calls). Ideally I would also like to enable resetMocks (and remove any explicit jest.resetAllMocks() calls). I'd like to evaluate use of restoreMocks once resetMocks is reached.

See jestjs/jest#10242 for more context

@JamieMagee JamieMagee added type:refactor Refactoring or improving of existing code status:requirements Full requirements are not yet known, so implementation should not be started priority-5-triage labels Nov 4, 2022
@viceice
Copy link
Member

viceice commented Nov 4, 2022

I'm ok with 1 but don't like 2 and 3. 2 and 3 are onyl for lazy people whow don't want to think about test order and side effects.
2 and 3 can also cause another performance penality and makes tests even slower than they already are.

@JamieMagee
Copy link
Contributor Author

I agree about the potential for performance penalties, but I think it's a trade-off between test correctness and performance. Already, enabling clearMocks, I see test failures due to tests unintentionally relying on setup from other tests.

@HonkingGoose HonkingGoose added priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:in-progress Someone is working on implementation and removed priority-5-triage status:requirements Full requirements are not yet known, so implementation should not be started labels Nov 7, 2022
@HonkingGoose
Copy link
Collaborator

Looks like PR #20402 fixes something about mocking as well:

Context

  • jest.resetAllMocks() now resets spy's, so don't reset if nor required
  • if a spy has no mock implementation, it calls the original

@JamieMagee JamieMagee mentioned this issue Apr 10, 2023
6 tasks
@rarkins rarkins added priority-4-low Low priority, unlikely to be done unless it becomes important to more people and removed priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Oct 1, 2023
@rarkins rarkins added status:ready and removed status:in-progress Someone is working on implementation labels Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-4-low Low priority, unlikely to be done unless it becomes important to more people type:refactor Refactoring or improving of existing code
Projects
None yet
Development

No branches or pull requests

4 participants