-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Parameterised mock return values #6180
Comments
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 |
I like it. |
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
If we do know the expect(mock).toHaveBeenCalledAllTimes();
I'm not sure what you mean here? The
Yup I love this 😄 Perhaps to make it explicit there could be a third API of jest.fn()
.when(x)
.thenThrow(() => throw new Error('💥')) |
After chatting with @rickhanlonii and @SimenB at the Jest summit we came up with the following new parameterised APIs:
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. |
Hi @mattphillips, any update on this? :) |
Hi! 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.
Which one is the return value? Which ones are the arguments? I think that Mockito's API is the cleanest:
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:
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:
Clean and Jesty :) Thoughts? |
As another data point, Jasmine has |
@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? |
@SimenB it looks like The When we chatted at the Jest summit, we decided to drop the intermediary chain of 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 Personally I prefer things to be more explicit. I'm definitely keen to hear what @idan-at @timkindberg think on the API design? |
haha, it's actually linked in this issue 😛 never noticed that... |
So I don't much like Java, but I did feel I was missing out on Mockito's I don't like the existing API suggestion of I think maybe something like this might be best: jest.mock().whenCalledWith(...args).mockReturnValue(someVal) I would have done this in 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"):
// Before (a regular mock)
jest.mock().mockReturnValue(true)
// 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: // 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 I see the desire to have nice short methods like |
I would think it just returns |
Just wanted to add... if you went with my proposal you'd only have to document one new method: Also if you didn't notice, fn.whenCalledWith(
expect.anything(),
expect.any(Number),
expect.arrayContaining(false)
).mockReturnValue('yay!')
const result = fn('whatever', 100, [true, false])
expect(result).toEqual('yay!') |
We already have: expect(mockFn).toHaveBeenCalledTimes(number) https://jestjs.io/docs/en/expect#tohavebeencalledtimesnumber |
@SimenB to be honest, I did not take part of I believe that the
The builder pattern is well known for its stateful behaviour, and using that for answering the question, it will simply be ignored. 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) |
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. |
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 |
Since jest doesn't support argument constraints on stubs (see jestjs/jest#6180), we use the 'jest-when' library to fake that certain input arguments are passed to the action by specifying that the 'core.getInput' function should return certain values when invoked with specific parameter names.
Hello. Wondering on any updates for these new APIs? |
<travolta meme here> |
Hi folks, this looks lovely. Some prior art to reference may be testdouble's api |
Bumping this thread, I feel this should be in the core |
Bumping this, it is a very useful feature. |
While I haven't though about this issue in a long time, I think I'm convinced by @timkindberg's argument in #6180 (comment)
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 |
@SimenB Yes I still think its a good solution. 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 Also the API would change, which I think is clear in my comments above. But just to reiterate, in jest-when we do I'd be happy to give a core dev a tour of jest-when if they are interested. |
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 // and jsdom
const NodeEnv = require('jest-environment-node');
module.exports = class NodeEnvWhen extends NodeEnv {
constructor(...args) {
super(...args);
this.moduleMocker = new JestWhenModuleMocker(this.global);
}
};
(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 This of course goes for all other approaches as well - good way to experiment with different APIs |
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 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 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 🥳 |
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. |
@nirga over a year later - please do! 😀 |
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 |
🚀 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 thewhen
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
andmockReturnValueOnce
.Example
The API in Mockito also offers argument matchers for given types and custom equality checkers i.e.
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?undefined
?cc/ @SimenB @rickhanlonii @cpojer
I'm happy to start on a PR for this if you guys are happy adding it to core 😄
The text was updated successfully, but these errors were encountered: