Jest: Mocking a module API pains

Created on 12 Apr 2018  路  21Comments  路  Source: facebook/jest

Firstly, I think jest is a great test runner and I appreciate all the hard work that has gone in to it. Awesome job 馃憤

However, the mock API does irk me. So I'd like to open this issue to see if there's something that we can do about it. Some documentation changes may help, some different APIs also may be beneficial.

Do you want to request a _feature_ or report a _bug_?
Feature

What is the current API?
Here are some of my pain points with the mocking APIs:


Unclear semantics

The difference in semantics between jest.mock and jest.doMock are not apparent from the names alone, I have to check the docs to remind myself. Same goes for jest.unmock and jest.dontMock. And jest.resetAllMocks and jest.restoreAllMocks. And mockFn.mockReset and mockFn.mockRestore. I think you get the picture...


jest.mock hoisting

jest.mock calls are hoisted to the top of the scope (via babel-jest). The issues I see here:

  1. Hoisting function calls to the top of the current scope is non-standard JS, it's a language extension. Users probably won't expect it, which may cause them problems until the new behaviour is learnt. For example:
    javascript // original import foo from './foo' jest.mock('./foo') foo() // is this call mocked?
    javascript // transpiled output jest.mock('./foo') import foo from './foo' foo() // yes! it is mocked.
  1. It feels like the hoisting was added to fit in with the way existing tests were written, rather than attempting to change the way people write tests to fit the mocking problem. I think TestDouble.js has an interesting solution to this problem. An obvious rip off of their API might look like:
    javascript let mockedFoo = jest.replace('./foo') let mockedBar = jest.replace('./bar').default // in this case bar is a default export let subject = require('./subject') // subject now depends on mocked foo and bar
    The CommonJS (CJS) module format is more flexible for messing around with modules than ES6 modules (ESM). I would suggest keeping both the current API for ESM and introducing the new for CJS tests and in some future version suggest new tests should be written in the CJS style. This way the hoisting behaviour, as well as some of the lesser known caveats of ESM, can be avoided.


Large API surface

The API relating to mocking a module as of v22.4.2 consists of:

  • jest.mock/jest.unmock
  • jest.doMock/jest.dontMock
  • jest.setMock
  • jest.genMockFromModule
  • import * as foo from './foo' + jest.spyOn (See #936)
  • Manual mocks
  • require.requireMock
  • jest.enableAutoMock/jest.disableAutomock

Are all of these different methods of mocking a module required? Can a simpler, smaller API be found to cover the above cases? I don't have any immediate answers but would love to see what the community come up with.


I can probably fix most of these by using a 3rd party lib but it would be nice if Jest made some changes so I didn't have to. I willing to accept that I hold a minority opinion on these APIs and if so, it may not be worth anyones time to work on this. I thought it was worth raising all the same.

Discussion

Most helpful comment

I second this. jest mock API is convoluted, at best, and generally confusing, IMHO.

  • Do I need to _reset_, _restore_ or _clear_ a mock?
  • Why do _restore_ work only on spies?
  • Is it a _module_ or a _mock_ that needs restoring?
  • Do I use genMockFromModule or plain .mock to mock a _module_?
  • Was it .mock that gets automatically hoisted... or is it doMock?
  • Why is there an unmock?
  • How do I partially mock a module's API?
  • Why is it not straightforward to declare mocks _per test_?

I'm not saying these questions don't have proper answers. I'm making the point that it's the fact that you need to consider those answers and think about them every time you (or at least I) write a test using jest. I end up losing myself in the docs and/or SO more than I feel I should.

All 21 comments

It's 1 AM here, so I'll save a larger response for later, but this article explains mocks pretty well: https://medium.com/@rickhanlonii/understanding-jest-mocks-f0046c68e53c

I do agree the API surface is probably larger than it has to be (and @cpojer has mentioned in the past that he'd like to revamp the API of it). Any concrete proposals (beyond the test double example)?

/cc @rickhanlonii

I second this. jest mock API is convoluted, at best, and generally confusing, IMHO.

  • Do I need to _reset_, _restore_ or _clear_ a mock?
  • Why do _restore_ work only on spies?
  • Is it a _module_ or a _mock_ that needs restoring?
  • Do I use genMockFromModule or plain .mock to mock a _module_?
  • Was it .mock that gets automatically hoisted... or is it doMock?
  • Why is there an unmock?
  • How do I partially mock a module's API?
  • Why is it not straightforward to declare mocks _per test_?

I'm not saying these questions don't have proper answers. I'm making the point that it's the fact that you need to consider those answers and think about them every time you (or at least I) write a test using jest. I end up losing myself in the docs and/or SO more than I feel I should.

@mattphillips where did the issue we worked on during the summit go?

That new proposal looks awesome 馃憤. Adding my two cents: it would actually be beneficial to users if all other functions/methods get deprecated rather than this new chainable API be built _on top_ or sit alongside them.

The aim of my new proposal is to add the new features alongside to get the feature out there as a minor release and then deprecate the old APIs in the next major release.

Regarding reset vs restore vs clear, I think we should just give them better names. clearInvocations, restoreOriginal, resetImplementations etc. More verbose, but you don't have to check the docs

And more clearly separate generic mock functions from module mocks would be good

Another honest concern that I can think of here is that, while this new API looks nicer and more user-friendly, does not actually address most of outlined problems regarding mocking _creation_.

At least in my experience, _writing_ a mock was never an issue.

// This:
const isOdd = n => n % 2 != 0;
const mock = jest.fn()
  .when(isOdd)
  .thenReturn('馃崒');

// Can be written now as:
const mock = jest.fn(n => isOdd(n) ? '馃崒' : undefined);

Which is arguably equally readable.

Defining the contents of the mock function (the argument to fn()) was never a pain point. Just take a look at TC's list.

Thoughts?

Hi all, I've been using jest for quite a while now. I agree with OP that the runner itself is fantastic. That said, I've tried a number of times over the many projects I've used jest on to use jest mocks. I always run into confusion and problems. It's very possible that I'm doing something wrong and have some fundamental misunderstanding, but the current API doesn't appear to lead to the pit of success.

I think that the problems I've run into boil down to a simple fact: jest mocks are not generally safe to use between tests. That is, they leak state between tests by default. As far as I can tell, they leak recordings and implementations. If I mockReturnValue in a test then the next test will get that return value too if I don't manually restore the mock.

This plays poorly with some of the other features of jest like manual mocks. Here's an example of something I'm trying to do right now that led me to this issue.

__mocks__/service.js

export default {
  foo: jest.fn(() => 'original')
}

__tests__/mytest.js

import service from '../service'

it('original', () => {
  expect(service.foo()).toEqual('original')
})

it('override', () => {
  trelloService.foo.mockReturnValue('override')
  expect(service.foo()).toEqual('override')
})

it('original', () => {
  // none of these matter:
  // jest.restoreAllMocks()
  // jest.resetAllMocks()
  // jest.clearAllMocks()
  expect(service.foo()).toEqual('original') // FAIL
})

So, I know I'm doing something "wrong" here. I probably shouldn't be using manual mocks in these ways. However, the problem doesn't end with manual mocks. Many of the jest examples in the docs show mocks being created at the module level. This fails too:

const foo = jest.fn(() => 'original')

it('original', () => {
  expect(foo()).toEqual('original')
})

it('override', () => {
  foo.mockReturnValue('override')
  expect(foo()).toEqual('override')
})

it('original', () => {
  expect(foo()).toEqual('original') // FAIL
})

Furthermore, the necessity for jest.mock to be at the module level makes it more likely that jest.fn()'s will be created at the module level. Basically, every time I've used jest I've felt pushed to create mocks at the module level only to find that, like the north, mocks remember.

What I think would be fantastic is if:

  1. There was a single function to completely reset mocks to the state they were in when they were created.
  2. That function worked on jest.fn()'s created at the module level (or in manual mocks).
  3. That function was called between tests by default.

I believe, if those were true, then mocks would "just work" and wouldn't leak state between tests.

The API mentioned above looks nicer, but, at least for me, this is the much bigger pain point.

Agree with most points raised by @aaronjensen above. The runner is phenomenal - but some design decisions baked into the API itself really irk me (and might not be alone here). To me, it seems as though Jest's API (not just mocks) really likes global state and hidden stuff:

  • All of its core depends on a foreign build tool (babel-jest).
  • All of its API is declared on the global object (never understood the reason behind neglecting imports).
  • Many non-obvious things can be happening in the background through config (like automatic reset of mocks).
  • Mocks remember state through tests.
  • Generally speaking, test _order matters_ when using mocks.
  • Mocks can be shared trough test suites.
  • Mocks are automatically "hoisted" so what you write is not necessarily what gets executed.

The API mentioned above looks nicer, but, at least for me, this is the much bigger pain point.

鈽濓笍This. I believe none of the listed points are really covered by the new design, unfortunately.

Thanks everyone, this is all great feedback, please keep it coming!

@nfantone @aaronjensen you both mentioned the mock state being remembered across tests. We have a config option to opt-out of this behavior: resetMocks. Turning this on will reset the mock calls and implementation (effectively re-declaring the mock) for every test. Does this fix that concern for you?

@rickhanlonii No, it doesn't, at least not for this:

const foo = jest.fn(() => 'original')

it('original', () => {
  expect(foo()).toEqual('original')
})

it('override', () => {
  foo.mockReturnValue('override')
  expect(foo()).toEqual('override')
})

it('original', () => {
  expect(foo()).toEqual('original') // FAIL
})

Likely because the arg passed to .fn is just shorthand for .mockImplementation. I'd expect it to be "stickier" than that so when a reset happens it'd reset to what I passed it originally.

@rickhanlonii Also, manipulating tests behavior through config always seemed arcane to me. I usually try to do without those kind of flags/switches that determine the outcome of expected results, despite having to be more verbose in my test files or write a bunch more code.

IMHO, an ideal runner would allow me to:

  • Import functions and matchers that I want to use to test a particular piece of functionality.
  • Declare mocks that are scoped to test blocks and do not leak state outside them.
  • Run tests independent of their order.
  • Copy a single test(...) and move to it to another file with no consequences.

Unit tests are built around the concept of _isolation_. I think we should be applying the same ideas to the _creation_ of tests.

Contributing to the discussion, I'm not quite sold on @aaronjensen expectations on the shown examples.

  • If you declare a mock function that's scoped to the entire module, I'd expect it to behave exactly like it does now. In my opinion, if you change its internal state, that's _on you_.
  • If you want different mocks for different tests, create them where they belong.
it('original', () => {
  const foo = jest.fn(() => 'original')
  expect(foo()).toEqual('original')
})

it('override', () => {
  const foo = jest.fn(() => 'override')
  expect(foo()).toEqual('override')
})

it('original', () => {
  const foo = jest.fn(() => 'original')
  expect(foo()).toEqual('original') // PASSES
})

I don't think there should be any "magic" involving mocks that would change their natural variable scope.

@nfantone I'd totally agree with you if it weren't for the rest of how jest works and what the examples in the documentation show.

The example you pulled from my post is the most contrived of all of the examples. The problem is complicated by jest.mock, which must (without some hoop jumping) be done at the module level. The API for jest.fn makes it appear as if you are setting the "default" state. For example:

import foo from 'foo'
jest.mock('foo', () => jest.fn(() => 'original'))

it('original', () => {
  expect(foo()).toEqual('original')
})

it('override', () => {
  foo.mockReturnValue('override')
  expect(foo()).toEqual('override')
})

it('original', () => {
  expect(foo()).toEqual('original') // FAIL
})

Suffers from the exact same problems as my original example. The only way to reset it would be to add a beforeEach with foo.mockImplementation(() => 'original'), in which case you can/should remove the argument to jest.fn. That is obviously a contrived example as typically you'd be importing from the actual module under test and testing that (that module would, in turn, import foo).

Also, see my example with the manual mocks. How would you create different mocks for different tests in that case? It is effectively not possible to create jest.fn with a default implementation that survives test runs and is temporarily overridable.

It boils down to defaults and documentation that lead to tests sharing state rather than the pit of success which would be completely isolated test doubles.

I really like the idea of keeping things at the module level as it avoids having to declare and assign lets like you would in testdouble but if the functionality doesn't actually work for it without bleeding state between tests, it really shouldn't be used in that way (or documented as such).

Perhaps related, it appears that there is actually a bug preventing jest.restoreAllMocks() and restoreMocks: true from working as intended: https://github.com/facebook/jest/issues/6059 https://github.com/facebook/jest/issues/5790 so it's possible that that is adding to the confusion.

Sorry to have only just caught up now. I'm glad to see other people are having similar trouble to me.

@SimenB @rickhanlonii I also agree with @nfantone and @aaronjensen, the new proposal looks cool but doesn't help these issues unfortunately.

@rickhanlonii I didn't know about the resetMocks config option. I don't think that will help, however the clearMocks option might. @aaronjensen does that cover the cases you've shared? If so, I think it should be enabled by default.

@nfantone Generally, I'd want to be able to create my mocks once and use them in multiple tests. I don't want the overhead of creating the mocks for a module in every it. The test runner already has some "magic" for each test and I think it's good and helpful if it's known and it's semantics are clear. Hence why I'm not a fan of jest.mock or the naming of various mock helper functions.

I want to work on a new API a bit, lacking time at the moment, perhaps end of this week or early next week

@aaronjensen does that cover the cases you've shared? If so, I think it should be enabled by default.

No, it does not. There is no setting afaict that causes jest.fn(...) to reset to the ... implemenation.

@aaronjensen You raise good points by pointing out how docs make things out to be. And yes, jest.mock is an entirely different beast. Have you tried mockImplementationOnce? I think it _might_ be a good fit for your example:

const foo = require('foo');
jest.mock('foo', () => jest.fn(() => 'default'));

it('original', () => {
  expect(foo()).toEqual('default');
});

it('override', () => {
  foo.mockImplementationOnce(() => 'override');
  expect(foo()).toEqual('override');
});

it('original', () => {
  expect(foo()).toEqual('default'); // PASSES
});

And since we are on the topic...

@nfantone Generally, I'd want to be able to create my mocks once and use them in multiple tests. I don't want the overhead of creating the mocks for a module in every it.

I was actually referring to jest.fn(...), but I understand where you're coming from. Broadly speaking, I'm on board with the idea of "create once and re-use" if it is _module_ mocking we are talking about. Unfortunately, in my experience, that kind of neat scenario doesn't come around too often. There's always this instance in which you would like to have a "default mock" implementation, but override it or make it behave differently for this one test. And jest API doesn't play very well with this kind of handling, unless you take very good care of crafting your tests and avoid falling into the pitfalls we are discussing here. My point being that "global module mocking" is only ever useful in contrived situations (http and other native modules mocking come into mind) and not generally applicable as they tend to favor state sharing and brake encapsulation.

Have you tried mockImplementationOnce? I think it might be a good fit for your example:

Yes, though that assumes that mock is only called the once in my test. That's often a safe assumption, but it's the kind of thing I don't want to have to think about. I typically would use a "once" method if I wanted a mock to only temporarily behave a certain way in the scope of a test. I realize that my mindset is clashing with the design of jest.mocks, but that's the point 馃槃

@aaronjensen Absolutely. Clashing is where we are at.

Incidentally -and I know it still suffers from having to do that mental exercise you were denouncing- but you can chain mockImplementationOnce calls if your test is expected to call your mock more than once.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nsand picture nsand  路  3Comments

jardakotesovec picture jardakotesovec  路  3Comments

StephanBijzitter picture StephanBijzitter  路  3Comments

stephenlautier picture stephenlautier  路  3Comments

paularmstrong picture paularmstrong  路  3Comments