Skip to content
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

stub_fetch clobbers jest-fetch-mock #7

Open
geelen opened this issue Jun 22, 2021 · 7 comments
Open

stub_fetch clobbers jest-fetch-mock #7

geelen opened this issue Jun 22, 2021 · 7 comments

Comments

@geelen
Copy link

geelen commented Jun 22, 2021

Just tried switching from service-worker-mock to egde-mock, but ran into trouble with me trying to capture and verify the subrequests going from the worker.

I've been using jest-fetch-mock, with the following line in jest.setup.js:

require('jest-fetch-mock').enableMocks()

No matter what I do, I can't seem to make it work with makeEdgeEnv, since that's injecting fetch in a way that breaks it. I've tried:

import mockFetch from 'jest-fetch-mock'

// ...
makeEdgeEnv({ fetch: mockFetch })

But I get:

    signal not yet implemented

      at EdgeRequest.get signal [as signal] (node_modules/edge-mock/src/models/Request.ts:70:11)
      at normalizeRequest (node_modules/jest-fetch-mock/src/index.js:119:15)
      at node_modules/jest-fetch-mock/src/index.js:76:20
      at node_modules/jest-fetch-mock/src/index.js:96:29

Weirdly, sometimes running the tests a second time (in Jest's watcher) causes them to pass, so something strange is going on with the global namespace. But wondering if we could figure out a way that if... globalThis.fetch already exists, say, don't inject it? Only inject your example.com/node-fetch one if it isn't?

Just FYI: I'm working on trying to upgrade worker-typescript-template to be a bit more modern (using module syntax for exports, in particular). Would love to get the testing story upgraded too. I've already got a TEST_ENV=edge mode that uses wrangler dev under the hood, which is pretty neat. But if we can make this library a drop-in for service-worker-mock I'd be super happy to use it.

@samuelcolvin
Copy link
Owner

samuelcolvin commented Jun 22, 2021

I would guess (without trying, but I will if this doesn't work), that something like this should work:

import {makeEdgeEnv} from 'edge-mock'
import mockFetch from 'jest-fetch-mock'

describe('whatever', () => {
  beforeEach(() => {
    makeEdgeEnv()
    mockFetch.enableMocks()
    jest.resetModules()
  })

  test('my-test', async () => {
    ...
  })
})

@samuelcolvin
Copy link
Owner

If it helps, you might want to have a look at https://github.com/samuelcolvin/edgerender/tree/main/tests, in particular handle.test.tx as it uses edge-mock.

@samuelcolvin
Copy link
Owner

We could also change the behaviours of makeEdgeEnv so something like the following stopped it from installing fetch into global?

import {makeEdgeEnv, SKIP_MOCK} from 'edge-mock'

...
makeEdgeEnv({ fetch: SKIP_MOCK })

PR welcome for this.

@samuelcolvin
Copy link
Owner

I'm assuming this is fixed, but feel free to ask more questions if not.

@geelen
Copy link
Author

geelen commented Jun 24, 2021

I'd almost go the other way, since the behaviour of your fetch mock is pretty narrow, I can see it needing to be configured:

import {makeEdgeEnv, stubFetch} from 'edge-mock'

makeEdgeEnv({ fetch: stubFetch() })

That'd make it really easy to use fetch_live, or to change the behaviour of stubFetch away from example.com:

import {makeEdgeEnv, stubFetch} from 'edge-mock'

makeEdgeEnv({ fetch: stubFetch({
  200: ['https://api.my-domain.com/*'],
})})

I think you could keep the same behaviour as-is, or actually have no default fetcher if you don't pass in a fetch argument.

In any case, passing fetch: null feels like it should have the semantics of SKIP_MOCK rather than a separate import?

@geelen
Copy link
Author

geelen commented Jun 24, 2021

This isn't fixed, btw. I've got a branch trying to get the two working together but no luck so far. Have put edge-mock aside for the moment and will come back to it

@samuelcolvin
Copy link
Owner

Maybe easiest just to remove the default mock of fetch.

For individual cases it's pretty easy to mock it yourself.

@samuelcolvin samuelcolvin reopened this Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants