feat: conditional mocking and other enhancements

[yinzara]

This incorporates idea from #121 and #102 but with a much more robust API that supports the concept of "once". Also adds enableMocks and disableMocks helper functions to make using the library easier. This also adds the ability to take the 'input' and 'init' arguments from 'fetch' in to the response promise functions. Finally it adds the ability for the response promise functions to return a string instead of an object with a body. I also added the prettier configuration that matches the 'index.d.ts' file so that those files can also be formatted with prettier.

I updated the README with all the details so I highly suggest heading over to my fork website and browsing it rendered.

• Add conditional mocking functionality
• Add response functions returning string support
• Add enableMocks and disableMocks functions
• Add optional response function input and init arguments

[jefflau]

Thanks so much for this! I will take a look

[jefflau]

Just had a quick run through of the code. Overall I think it looks good. Couple of questions:

If I run onlyMockOnceIf twice in a row, will they mock the next to a row and then go back to default? I didn't see any tests with chaining these together. Same question for doMockOnce and dontMockOnce or any combinations of these once operations. If so I'd like to see tests combining them in more complicated ways to make sure the API is robust

[yinzara]

You're absolutely right I probably should add some more complex chaining rules.

The new "mock if" feature is implemented using a single jest mock attached to 'fetch.isMocked', just like the primary mocked fetch function.

That mock is always called exactly once for every call to the primary fetch mock (as the primary mock always eventually calls through to the normalizeResponse function which calls the 'isMocked' function).

All of the "mock once" functions really just call "mockImplementationOnce" on the "isMocked" function so you can chain any combination of them together just like you can change together "mockResponseOnce" however any call to any of the other "do/dont/only/never" functions (i.e. the ones without 'once') will reset the behavior because they call "mockImplementation" on "isMocked".

I did this to keep with the original spirit of the libraries "once" functionality.

This would allow you to do something like:

fetchMock.neverMockIf("http://foo")
    .onlyMockOnceIf("http://foo/bar")
    .doMockOnce()
    .neverMockOnceIf("http://bang")

which would say "on the first call to fetch, if the url matches "http://foo/bar", mock the response, otherwise dont mock. On the 2nd call, if the url matches "http://foo/bar" don't mock the response, otherwise mock the response. On the 3rd call, mock all responses. for subsequent calls, if the request url matches "http://foo", don't mock the response, otherwise mock the response"

[jefflau]

Just to clarify your example.

1) If the first mock is "http://blah" should this mock with the default ""? 2) and then the second is "http://foo/bar" it should also mock with the default ""? (because the first once was 'used' already by "http://foo"

Is that a correct assumption? I think this could get pretty confusing so we need to iron out how we document this as well and probably leave it as an advanced use case.

Completely unrelated to this, but I can see you used a spy to replace the original fetch in the tests so it returns bar. I've been thinking about how to implement a 'don't mock, just run the fetch' functionality as it has been requested a couple times, although it's not a high priority. The reason why I bring it up is because I think the neverMock function makes it sounds like it's going to just pass through the fetch so it uses the original request - is that true or is it just defaulting to the empty string mock?

[yinzara]

When I say "passes though" or "does not mock", I mean that it will use the real fetch implementation and not the mock implementation (i.e. it's as if the mock library wasn't being used). This allows to selectively mock things while letting other requests actually go to the real fetch back end.

I added a spy in my test case because I needed to test it going through to the real fetch back end (i.e. using cross-fetch).

Let me add a few complex tests that combine "mockResponse" "once" and my new methods to illustrate.

[yinzara]

I've now added test cases to cover all varieties of combinations using the mock once functions with 'once' and a nice example of using the request uri and an async function to drive which mock response is returned.

[jefflau]

The conditional mock functions cause jest-fetch-mock to pass fetches through to the concrete fetch implementation conditionally. Calling fetch.dontMock, fetch.doMock, fetch.onlyMockIf or fetch.neverMockIf overrides the default behavior of mocking/not mocking all requests. fetch.dontMockOnce, fetch.doMockOnce, fetch.onlyMockOnceIf and fetch.neverMockOnceIf only overrides the behavior for the next call to fetch, then returns to the default behavior (either mocking all requests or mocking the requests based on the last call to fetch.dontMock, fetch.doMock, fetch.onlyMock and fetch.neverMock).

I think this is making more sense now. According to this part of the readme the way it works is, if you don't call dontMock, the default across all mocks is the empty string. If you do call dontMock and then use the original once, it will mock the next function and then it will let the next request use the backend (i.e. no mock at all).

Sorry for all the back and forth, just trying to make your new feature as easy as possible for other people to use. I think for the README the complicated example can be left for reference, but we should add some conditional, but simple examples to allow people to gradually get used to the new features e.g:

• dontMock + once => first request mocked, second request goes to backend
• dontMock + mockOnceIf => first request mocked, second request mocked because URL predicate matches
• dontMock + mockOnceIf => first request Mocked, second request not mocked because URL predicate fails match
• mockResponses + dontMockOnce => first request mocked by mockResponses, second request goes to backend, third request mocked by mockResponses
• An example of how to use isMocking

Also can you please add a link in the contents to your conditonal mocking section. I would probably split it up into conditional mocking - dontMock + once, conditional mocking - dont mock + mockOnceIf to make it easier for people to cycle through your example.

One last question to clarify high level functionality. If I call dontMock and then I call mockOnceIf. Then the first request I do does not match mockOnceIf, if I put in a second request that matches the URL, will it be mocked? I'm assuming no if it matches the way once works. If so we should make that clear in the README as well as I feel once we get into conditional mocking, it's an easy to mistake for people who have never used jest-fetch-mock before.

Again - thanks a lot for putting in all this effort. I've wanted to do this PR for a while, but never had the time.

[yinzara]

Apologies. Work's been getting in the way. I'll take a look at this today.

[yinzara]

After rereading your comment above, I think there is still confusion about how the new feature works.

The mock used for the original "mockResponse/once" is a different mock than the one used for the "dont/do/only/if" mock functions.

That means that in your above examples: dontMock + once => this would cause all requests not to be mocked since you called "dontMock" then didn't call any version of "doMockOnce/onlyMockOnce/neverMockOnce" (any of which may cause the next request to be mocked if it matches the criterion).

To get the result you're suggesting you would need: dontMock + doMockOnce + once => mock the next response with the value from "once" then don't mock any responses after the first This would have the same result as: dontMock + doMockOnce + mockResponse OR mockResponse + dontMock + doMockOnce

However: mockResponse + doMockOnce + dontMock => would cause all responses to not be mocked since "dontMock" was called after "doMockOnce" which changes the default behavior and "undoes" any "once" just like calling "once + mockResponse" would ignore the value passed to "once" and mock all responses with the value passed to mockResponse

You could even string together the onces mixed up or not: dontMock + doMockOnce + doMockOnce + once + once OR dontMock + doMockOnce + once + doMockOnce + once => mock the next two responses with the values passed to each call to once then stop mocking

[jefflau]

Ah this is confusing and what I want to avoid in the API as much as possible.

So if I want the first example dontMock + once

to work, I need to call doMockOnce AND then called once? The doMockOnce is just allowing the possibility of a once being called later

Do you think it might be cleaner if we just have doMockOnce take the arguments for once and pass it through so you don't have to call two different methods. Then the documentation can be very clear and say. If you use dontMock, you should use the do version.

Infact thinking about it now, is it possible to have the api mirror what I mentioned, but instead of having doMockOnce, we inject that behavior into once and allow it to overwrite any dontMock just for that one call.

[yinzara]

So I think you're overcomplicating things by trying to link together the original API (i.e. mockResponse, mockResponseOnce (i.e. "once)) with the conditional mocking API.

They're intended to be separated because in general they're used for different purposes. They both function in identical ways in the "once" manner so that there is less confusion on how they work.

The mockResponse API is to say "what" should be returned "if and only if the response of the API is mocked". "mockResponse" changes the default response and "mockResponseOnce" changes the next call to fetch if and only if that request is mocked (which in prior versions was always true).

The PR only changes "if" it should be mocked, not what it should return.

The "only/never/do/dont" mock APIs indicate "if the response should be mocked". "only/never/do/dont"Mock changes the default behavior of the API to mock based on which was called (just like "mockResponse). The "only/never/do/dont"MockOnce changes the behavior of the next fetch (just like "once").

Right now "mockResponseOnce" is just an alias for "once". Maybe it should be changed that "once" is a shortcut of the combo of "doMockOnce" and "mockResponseOnce" to allow for the behavior you're describing? In the legacy case where you didn't call any of the do/dont/only/never mock APIs, it will still behave as it does now (i.e. mocking all calls).

In the end, everything revolves around "fetchMock.isMocking" (which is the true/false mock fn that determines if the fetch being made is mocked). Just like all the original APIs are based around the mock fn "fetch".

[jefflau]

@yinzara Sorry for the late response, I've been super busy with a conference and travelling.

Maybe it should be changed that "once" is a shortcut of the combo of "doMockOnce" and "mockResponseOnce" to allow for the behavior you're describing?

I think this makes a lot of sense, as doMockOnce makes it sound like it's going to mock, not just 'allow for the mock to happen'. At the end of the day, I don't want to overcomplicate things, but as the original developer, if I'm confused, I'm sure it will be confusing for others!

[yinzara]

Alrighty I've updated so that "once" now is a combination of "mockResponseOnce" and "doMockOnce". That makes its behavior like you describe above. I've updated the README to call out its difference from the "mockResponse" and added tests to verify it behaves as expected.

Btw, I get a dtslint error that says I have to upgrade my TypeScript header in the index.d.ts to 3.0 . I didn't make any material changes to this so I'm surprised at the error.

[yinzara]

Actually I figured it out. The optional arguments specified in the extends statement of FetchMock aren't supported until TypeScript 3.0. You can change them to " | undefined" to support the earlier versions.

[jefflau]

I merged your PR into a local branch so I could test locally and make any README adjustments. I just realised one more thing I'm not sure about:

- `fetch.onlyMockIf(urlOrPredicate):fetch` - causes all fetches to be not be mocked unless they match the given string/RegExp/predicate
  (i.e. "only mock 'fetch' if the request is for the given URL otherwise, use the real fetch implementation")
- `fetch.neverMockIf(urlOrPredicate):fetch` - causes all fetches to be mocked unless they match the given string/RegExp/predicate
  (i.e. "never mock 'fetch' if the request is for the given URL, otherwise mock the request")
- `fetch.onlyMockOnceIf(urlOrPredicate):fetch` - causes the next fetch to be mocked if it matches the given string/RegExp/predicate. Can be chained.
  (i.e. "only mock 'fetch' if the next request is for the given URL otherwise, use the default behavior")
- `fetch.neverMockOnceIf(urlOrPredicate):fetch` - causes the next fetch to be not be mocked if it matches the given string/RegExp/predicate. Can be chained.
  (i.e. "never mock 'fetch' if the next request is for the given URL, otherwise use the default behavior")

Shouldn't these also take a response object? I was looking through the README and the tests but couldn't find how to actually give these conditional mocks a response. Are they supposed to be chained with doMockOnce - but that also doesn't take a response? I am thinking these should also call mockResponseOnce themselves and pass through the response.

I think bottom line, people that are going to want to mock based on URL, will also want to give a specific response to that URL

[jefflau]

What do you think of this kind of API?

//default is mocked using fetch.mockResponse(mockedDefaultResponse)
it('mocks when matches string', async () => {
      const response = 'blah'
      const response2 = 'blah2'
      fetch
        .onlyMockOnceIf('http://foo', response)
        .onlyMockOnceIf('http://foo2', response2)
      await expectMocked('http://foo', response)
      await expectMocked('http://foo2', response2)
      await expectMocked('http://foo', mockedDefaultResponse)
})

I just added this to onlyMockOnceIf

fetch.onlyMockOnceIf = (urlOrPredicate, bodyOrFunction, init) => {
  isMocking.mockImplementationOnce(requestMatches(urlOrPredicate))
  mockResponseOnce(bodyOrFunction, init)
  return fetch
}

Although this doesn't seem to work the way I intended if I throw in the wrong URL:

//default is mocked using fetch.mockResponse(mockedDefaultResponse)
 it('mocks when matches string', async () => {
      const response = 'blah'
      const response2 = 'blah2'
      fetch
        .onlyMockOnceIf('http://foo', response)
        .onlyMockOnceIf('http://foo2', response2)
      await expectMocked('http://foo', response)
      await expectMocked('http://foo3', mockedDefaultResponse)
    })

Expected this to go back to the mockedDefaultResponse, but I get "REAL FETCH RESPONSE" instead.

[jefflau]

Okay, I think I finally get what's going on. You always need to add a once or mockResponseOnce after calling any onlyMockIf or onlyMockOnceIf

so the correct implementation (without my suggested changes) would be:

fetch
        .onlyMockOnceIf('http://foo')
        .once(response)
        .onlyMockOnceIf('http://foo2')
        .once(response2)
await expectMocked('http://foo', response)
await expectMocked('http://foo2', response2)

I am still getting this problem (with your implementation of onlyMockOnceIf) where if I add one more expect, I'm expecting it to be the defaultMockedResponse, but it ends up being the realResponse

it('mocks when matches string', async () => {
      const response = 'blah'
      const response2 = 'blah2'
      fetch
        .onlyMockOnceIf('http://foo')
        .once(response)
        .onlyMockOnceIf('http://foo2')
        .once(response2)
      await expectMocked('http://foo', response)
      await expectMocked('http://foo2', response2)
      await expectMocked('http://foo3', mockedDefaultResponse) //expecting to be mocked, but is the spy value `realResponse`
    })

I feel we're really close, thank you again for putting in all this time, I'm going to try and get this out early next week and bump it up a major version

[yinzara]

Love the idea

I think there are two options on how this could be implemented.

1. make it an optional parameter (i.e. bodyOrFunction) so only call "mockResponseOnce" if bodyOrFunction !== undefined. This makes a lot of sense and is how you have implemented it.

2. make it an "options" object that has a "response" and "init"/responseInit parameters. This provides for future flexibility

You could also do the same thing to all of the existing "mockResponse" functions as well (a "MockResponseOptions" with "onlyIf","neverIf"

I'll give you write access to my fork so you can push your changes.

[yinzara]

You're confusion with your second example is because of your request to change the "once" function to call "mockResponseOnce(args).doMockOnce() so you can rewrite your example as:

      const response = 'blah'
      const response2 = 'blah2'
      fetch
        .onlyMockOnceIf('http://foo')
        .doMockOnce()
        .onlyMockOnceIf('http://foo2')
        .doMockOnce()

     fetch
        .mockResponseOnce(response)
        .mockResponseOnce(response2)

      await expectMocked('http://foo', response)
      await expectMocked('http://any.url', response2)
      await expectMocked('http://foo2', mockedDefaultResponse)
      await expectMocked('http://any.other.url', mockedDefaultResponse)
    })

To get what you were explaining you would:

it('mocks when matches string', async () => {
      const response = 'blah'
      const response2 = 'blah2'
      fetch
        .onlyMockOnceIf('http://foo')
        .mockResponseOnce(response)
        .onlyMockOnceIf('http://foo2')
        .mockResponseOnce(response2)
      await expectMocked('http://foo', response)
      await expectMocked('http://foo2', response2)
      await expectMocked('http://foo3', mockedDefaultResponse)
    })

If we make each of the only/never/do functions to take in options or 2nd and 3rd arguments of the bodyOrFunction and responseInit then we give people the ability to do both operations in a single call.

Maybe we should make "once" return to the original implementation and just change "doMockOnce" to take in the options/bodyOrFunctions/responseInit?

I will say the "neverMockOnceIf('http://foo', 'response')" is a bit confusing :): If the next URL to be requested matches "http://foo", pass through to the real fetch and retrieve from that URL, otherwise return the given 'response'

Maybe adding some examples of situations you might use each function would be helpful.

"onlyMockIf" is best used where you have a specific URL or set of URLs you want to mock possibly with some kind of router function based on the request. All other URLs (which you might not even know them) should use the real fetch implementation.

"neverMockIf" is the opposite of "onlyMockIf". I would see the "neverMockIf" used in the case where you know there is a specific url or set of URLs that you want to "whitelist" that will always use the real fetch implementation, while you want all other responses to be mocked (possibly via a router type function using the request object to match). However you may not know all the URLs that might be accessed during the test and still want those to be mocked.

I would see the "dontMocK", "doMock", "onlyMockIf" or "neverMockIf" to be called in some kind of "beforeEach" (and only one of them as they would override each other) then possibly "dontMockOnce", "doMockOnce", "onlyMockOnceIf" and/or "neverMockOnceIf" during the test cases.

[jefflau]

Instead of you giving me permission on yours, I'll just give you permission to my repository! Let's work on this together.

Another thing is, we've got neverMock and dontMock I think we should just change this to dontMock and also remove only, so it's mockOnceIf and dontMockOnceIf, or alternatively doMockOnceIf and dontMockOnceIf, but I don't think neverMockOnceIf makes much sense with never and `once together

[yinzara]

That makes sense to me! Thanks! I'm happy to be a contributor.

[yinzara]

Closing in favor of PR #128