knee-cola / jest-mock-axios

Axios mock for Jest
252 stars 42 forks source link

getReqByUrl needs to fail if there is no matching request #42

Closed ThiefMaster closed 4 years ago

ThiefMaster commented 4 years ago

Let's say I have this code in my test:

mockAxios.mockResponse({...}, mockAxios.getReqByUrl('invalid.url'));

This will be equivalent to

mockAxios.mockResponse({...}, undefined);

so it will silently respond to the latest request, which may be the correct one - but the fact that it's not the URL I specified will be lost.

I think getReqByUrl should return something that's not undefined or throw an error when there's no matching request. For the sake of backwards compatibility, a mustGetReqByUrl or optional strict argument to getReqByUrl could be added.

kingjan1999 commented 4 years ago

That's a reasonable suggestion - thank you! Feel free to submit a PR (but don't feel obligated - I will surely implement this in the next days, if you don't beat me to it)

ThiefMaster commented 4 years ago

Any preference on the argument vs a separate function?

kingjan1999 commented 4 years ago

Hey,

I just gave this another thought: While I agree that this snippet:

const req = mockAxios.getReqByUrl('invalid.url');
mockAxios.mockResponse({...}, req)

responding to the last request exhibits an unexpected behavior (which could be simply circumvented by adding something like expect(req).not.toBeUndefined()), I don't think this is because of getReqByUrl returning undefined. I would consider returning undefined the more appropriate response than throwing an error for JS-developers (as it's done by e.g. Array.find, which is in fact used here)

However, the fact that mockResponse simply responds to the last request if a specific request is given, but undefined, instead of throwing an error is much more unexpected from my point of view. While this could be circumvented as well utilizing the silentMode (not tested):

mockAxios.mockResponse({...}, req, true)

changing this behavior seems desirable to me. Unfortunately, I don't know of any good way to do this as JS doesn't allow us to differentiate between explicitly passed undefined arguments and undefined arguments because no value was passed.

Any thoughts?

ThiefMaster commented 4 years ago

Hm, what about adding a mockResponseFor function that combines the two?

mockResponseFor('url', response) for simple cases, or mockResponseFor({url, method, ...}, response) as a more advanced version when you want to specify more than just the URL to find the request

kingjan1999 commented 4 years ago

Sounds great to me!