jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.22k stars 6.46k forks source link

Parameterised mock return values #6180

Open mattphillips opened 6 years ago

mattphillips commented 6 years ago

🚀 Feature Proposal

Add .when/.thenReturn support to the Jest mock API.

when: Takes arguments to match the mock call against. thenReturn: Takes a vale to return when the when clause matches a given call.

Motivation

This behaviour exists in mocking libraries from other languages see Mockito

This API will allow more expressive mocks, extending on top of the idea of mockReturnValue and mockReturnValueOnce.

Example

test('Mock when(x).thenReturn(y)', () => {
  const mock = jest.fn()
    .when(1) // one primitive argument
    .thenReturn(2)
    .when(3, 4) // multiple primitive arguments 
    .thenReturn(7)
    .when('id')
    .thenReturn(a => a) // return a function
    .when({ hello: 'world' }) // object argument
    .thenReturn('Hello, world!');

  expect(mock(1)).toBe(2);
  expect(mock(3, 4)).toBe(7);
  expect(mock('id')('Awesome Jest')).toBe('Awesome Jest');
  expect(mock({ hello: 'world' })).toBe('Hello, world!');
});

The API in Mockito also offers argument matchers for given types and custom equality checkers i.e.

test('returns jest-cool when given a number)', () => {
  const mock = jest.fn()
    .when(any(Number))
    .thenReturn('jest-cool');

  expect(mock(1)).toBe('jest-cool');
  expect(mock(2)).toBe('jest-cool');
  expect(mock(3)).toBe('jest-cool');
});

test('returns 🍌 when given odd numbers)', () => {
  const isOdd = n => n % 2 != 0;
  const mock = jest.fn()
    .when(isOdd)
    .thenReturn('🍌');

  expect(mock(1)).toBe('🍌');
  expect(mock(3)).toBe('🍌');
  expect(mock(5)).toBe('🍌');
});

Pitch

Why in Core?

This could go in user land but the API would suffer for it! It would be more verbose and would not be able to bind to the mock. Instead it would have to mutate Jest's mock API which would likely discourage people from using it.

My Questions

What should happen if a mock call value doesn't match any when clauses?

cc/ @SimenB @rickhanlonii @cpojer

I'm happy to start on a PR for this if you guys are happy adding it to core :smile:

SimenB commented 6 years ago

This would be incredibly awesome, and really empower mock functions in jest.

A couple of questions come to mind, more as discussion points than anything else.

Also, both sinon and testdouble has something like this. Would be great to look at their APIs for inspiration, and maybe look through their issue tracker for foot guns and missing features people have encountered

cpojer commented 6 years ago

I like it.

mattphillips commented 6 years ago

@SimenB

I wonder if this needs a times argument?

Ooo I like it! I'm not sure how we would add it to the API without it being a bit janky.

In my head we have .when(...args) so we can't put it in the function signature, unless times was the first argument meaning we can't put any defaults in. Do you have any suggestions?


Any elegant way to assert that all cases were hit?

If we do know the times all of the when clauses should be called we could add a new matcher, how about something like:

expect(mock).toHaveBeenCalledAllTimes();

How would we differentiate between a chained when and actual .when properties? Making it a factory comes to mind, but that's smelly

I'm not sure what you mean here? The when will be on our jest.fn() object along with other properties like mock, getMockImplementation, etc


Should it have throwing in addition to returning?

Yup I love this :smile:

Perhaps to make it explicit there could be a third API of .thenThrow(fn), for example:

jest.fn()
  .when(x)
  .thenThrow(() => throw new Error('💥'))
mattphillips commented 6 years ago

After chatting with @rickhanlonii and @SimenB at the Jest summit we came up with the following new parameterised APIs:

New parameterised API Existing API
returnWhen(value, ...args) mockReturnValue(value)
resolveWhen(value, ...args) mockFn.mockResolvedValue(value)
rejectWhen(value, ...args) mockFn.mockRejectedValue(value)
throwWhen(value, ...args) Needs creating

The new APIs would allow us to write a mock like so:

const mock = jest.fn()
  .returnWhen(3, 1, 2)
  .resolveWhen(3, 3)
  .rejectWhen(new Error('Async error'), -2)
  .throwWhen('Error', -1);

TODO:

I'm going to start working on this tonight/tomorrow and will have a PR open soon.

cedric25 commented 6 years ago

Hi @mattphillips, any update on this? :)

myoffe commented 6 years ago

Hi! Any updates?

I would love to help making this happen. This is one of my only gripes with Jest.

It seems there's an implementation of this in these projects:

Also, I'm not thrilled with the new API. It's less readable when the return value and the arguments are written in the same context next to each other.

fn.returnWhen(10, 5, 2);

Which one is the return value? Which ones are the arguments? It's not immediately obvious because of the lack of separation.

I think that Mockito's API is the cleanest:

when(mockFn(1, 2)).thenReturn('yay!');

But I assume this is too far from Jest's current API design (although I would love for Jest to follow suite), and requires changing how function calls are intercepted.

jest-when has this:

when(mockFn).calledWith(1, 2).mockReturnValue('yay!')
when(mockFn).expectCalledWith(1, 2).mockReturnValue('yay!') // with assertion that a call was made

Which I think is pretty clean.

If wrapping the mock is still too different than the rest of the API, then maybe something like this, borrowing from SinonJS:

mockFn.withArgs(1, 2).mockReturnValue('yay!')

Clean and Jesty :)

Thoughts?

SimenB commented 6 years ago

As another data point, Jasmine has withArgs: https://jasmine.github.io/api/edge/Spy.html#withArgs

SimenB commented 5 years ago

@mattphillips check this one out: https://www.npmjs.com/package/jest-when. Seems very similar to our proposal here? It looks super awesome, but I haven't tried it out

/cc @idan-at @timkindberg thoughts on the API and suggestions in this thread?

mattphillips commented 5 years ago

@SimenB it looks like jest-when has come on quite a bit since I last checked it out last year!

The jest-when API appears to be closer to my original proposal of chaining when(x).then(y) or the withArgs API in Jasmine.

When we chatted at the Jest summit, we decided to drop the intermediary chain of when/withArgs and have a single API that includes both the args and value to be returned/resolved/rejected/thrown.

I think it's worth getting a consensus on which API we prefer.

I can see pros/cons for both, having one API for both feels more concise but is probably a little more confusing due to argument order mattering.

The when(x).then(y) style is certainly more explicit but what happens for trailing whens without closing thens and is obviously more verbose (possibly a moot point)?

Personally I prefer things to be more explicit.

I'm definitely keen to hear what @idan-at @timkindberg think on the API design?

SimenB commented 5 years ago

haha, it's actually linked in this issue 😛 never noticed that...

timkindberg commented 5 years ago

So I don't much like Java, but I did feel I was missing out on Mockito's when().thenReturn() while testing JavaScript. So I created jest-when as a spiritual port of mockito's when capability.

I don't like the existing API suggestion of fn.returnWhen(10, 5, 2); for the same reasons @myoffe mentions.

I think maybe something like this might be best:

jest.mock().whenCalledWith(...args).mockReturnValue(someVal)

I would have done this in jest-when but didn't want to decorate the global jest.mock object.

This will stay consistent with terms that jest already uses, there's no need to introduce new words and phrases to the API (other than "when"):

// After (you realize you'd like to parameterize it, just insert whenCalledWith()) jest.mock().whenCalledWith('foo').mockReturnValue(true)


It's best to keep one method for defining arguments and one method for defining what to return. This way they can easily be swapped, like so:
```js
// Before (a regular when mock)
jest.mock().whenCalledWith('foo').mockReturnValue(true)

// After (you realize you actually need a resolved promise returned, just swap for `mockResolvedValue`)
jest.mock().whenCalledWith('foo').mockResolvedValue(true)

I'm biased but I recommend going with the jest-when api with the only change being do not use a wrapper when function; instead convert the calledWith method name to whenCalledWith for clarity.

I see the desire to have nice short methods like returnWhen but the nice-short-method-names ship sailed the minute jest introduced mockReturnValue. So I think it's more important to keep consistent with the existing nomenclature.

timkindberg commented 5 years ago

but what happens for trailing whens without closing thens

I would think it just returns undefined, since that's the default return value for any mock.

timkindberg commented 5 years ago

Just wanted to add...

if you went with my proposal you'd only have to document one new method: whenCalledWith. You'd just have to describe how it can be inserted before any existing mock*Value method.

Also if you didn't notice, jest-when supports jest matchers as parameters which is also balanced with the assertion API. We could do that too like this:

fn.whenCalledWith(
  expect.anything(),
  expect.any(Number),
  expect.arrayContaining(false)
).mockReturnValue('yay!')

const result = fn('whatever', 100, [true, false])
expect(result).toEqual('yay!')
timkindberg commented 5 years ago
  • I wonder if this needs a times argument?

We already have:

expect(mockFn).toHaveBeenCalledTimes(number)

https://jestjs.io/docs/en/expect#tohavebeencalledtimesnumber

idan-at commented 5 years ago

@SimenB to be honest, I did not take part of jest-when's design, I've only recently contributed to it, because I think it's awesome :)

I believe that the withArgslike API has the most pros, it's very explicit and won't confuse new comers. (and it can be called whenCalledWith, as @timkindberg suggested - its better than withArgs). The major pros here is minimal new API, very explicit, and support for mock once (using mockResolvedValueOnce).

The when(x).then(y) style is certainly more explicit but what happens for trailing whens without closing thens and is obviously more verbose (possibly a moot point)?

The builder pattern is well known for its stateful behaviour, and using that for answering the question, it will simply be ignored. 
Keep in mind we should address the case of multiple whens and a single then. I think this should not be supported, since async matchers can solve that.

So eventually the API will look like:

jest.fn()
  .whenCalledWith(expect.any(String)).mockReturnedValueOnce(1) // on first call with string, return 1
  .mockReturnedValue(42) // on any other call (second with string, first with anything which isn't a string, return 42)
venikx commented 5 years ago

Another good reference for that type of behavior is testdoublejs. They have a very specific usecase for mocks and the usecase is from what I gather exactly what is being proposed here.

Might be worth checking out for API ideas.

davidje13 commented 5 years ago

Looks like the current thinking around this API is quite different than what's currently shipped, so my suggestion in #7943 probably isn't very relevant, but with this new API it will be even more important to support thenCallThrough (or something to that effect), which will achieve the same goal.

lgc13 commented 4 years ago

Hello. Wondering on any updates for these new APIs?

glebsts commented 4 years ago

\<travolta meme here> Guys, anyone? Started using jest heavily and didn't find this functionality, but found this thread.

tim-evans commented 4 years ago

Hi folks, this looks lovely. Some prior art to reference may be testdouble's api

Sytten commented 4 years ago

Bumping this thread, I feel this should be in the core

waseem commented 4 years ago

Bumping this, it is a very useful feature.

SimenB commented 4 years ago

While I haven't though about this issue in a long time, I think I'm convinced by @timkindberg's argument in https://github.com/facebook/jest/issues/6180#issuecomment-455018418

if you went with my proposal you'd only have to document one new method: whenCalledWith. You'd just have to describe how it can be inserted before any existing mock*Value method.

This sounds very compelling to me. @timkindberg what do you think almost 2 years later?

/cc @cpojer @thymikee @jeysal @mattphillips @rickhanlonii thoughts on potentially just adopting whenCalledWith from jest-when?

timkindberg commented 4 years ago

@SimenB Yes I still think its a good solution. jest-when is still an active library with decent usage. Just got some PRs this week in fact.

Only issue I see is maybe the way I've gone about writing jest-when is more monkey-patch or hack than actual implementation. I wonder how the code might change if it was able to be embedded within jest's actual source code. I have not spent much time looking at the jest codebase to know if the code from jest-when is transferrable or if it would be rewritten.

Also the API would change, which I think is clear in my comments above. But just to reiterate, in jest-when we do when(spyFn).calledWith(...).mockReturnValue(...) but the api in jest would be better as spyFn.whenCalledWith(...).mockReturnValue(...).

I'd be happy to give a core dev a tour of jest-when if they are interested.

SimenB commented 4 years ago

Cool, thanks @timkindberg! One thing you could do to re-create the API we'd have in Jest would be to extend from the default jest environment but override this.moduleMocker.

// and jsdom
const NodeEnv = require('jest-environment-node');

module.exports = class NodeEnvWhen extends NodeEnv {
  constructor(...args) {
    super(...args);

    this.moduleMocker = new JestWhenModuleMocker(this.global);
  }
};

jest.fn is essentially just environment.moduleMocker.fn.bind(environment.moduleMocker), see https://github.com/facebook/jest/blob/132e3d10068834e3f719651cdc99e31b7c149f3b/packages/jest-runtime/src/index.ts#L1526-L1527

(It'll be weird with legacy fake timers if you rely on them being mocked, but that's an edge case I think)

That way you wouldn't need the wrapper function. Might be a good way for people to play with the API suggested here? Not that wrapping in a when is super painful, of course.


This of course goes for all other approaches as well - good way to experiment with different APIs

timkindberg commented 4 years ago

I definitely do not follow completely. I think you are saying this is a pattern that could be followed to "try the api out"?

Wouldn't someone just add the whenCalledWith functionality straight into the jest-mock package as a method on the fn?

I'm looking at this file: https://github.com/facebook/jest/blob/132e3d10068834e3f719651cdc99e31b7c149f3b/packages/jest-mock/src/index.ts#L662

And it's doing a lot of the same stuff we did in jest-when where it all eventually funnels to mockImplementation. Everything is just sugar on top. So that looks a bit familiar.

If I were to work on the PR I'd need a bit more direction I think.


Side Announcement: jest-when has recently been added to the Adopt portion of the Thoughtworks Tech Radar 🥳
https://www.thoughtworks.com/radar/languages-and-frameworks?blipid=201911030

nirga commented 3 years ago

Anyone currently working on it? If not, I’d love to take a shot with a PR. Been using Mockito on Java for long and recently started using jest and I’m really missing this.

SimenB commented 2 years ago

@nirga over a year later - please do! 😀

sekoyo commented 2 years ago

Is there currently not a way to return different mocked values depending on args 🤯, or is this a proposal for something more ergonomicc? What's the current solution, it seems like a basic necessity so there must be something?

Edit: seems like jest-when package is the solution