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

Add Support for Mocking Requests using Undici MockAgent #293

Merged
merged 6 commits into from
Aug 18, 2022

Conversation

william1616
Copy link
Contributor

@william1616 william1616 commented Jun 20, 2022

Hello,

I don't know what the contributor guidelines for miniflare are but it's a cool project and this is a simple workaround for #162 so hopefully this change can be accepted

We use cloudflare workers to write API gateways and we love it but testing has always been a nightmare; miniflare seams like a great way for us to do testing but it's missing one important thing - Fetch Mocks

This PR adds three new functions to Miniflare

createMockAgent which wraps undici MockAgent constructor - https://github.com/nodejs/undici/blob/main/docs/api/MockAgent.md#new-mockagentoptions
getGlobalDispatcher which wraps undici getGlobalDispatcher - https://github.com/nodejs/undici#undicigetglobaldispatcher
setGlobalDispatcher which wraps undici setGlobalDispatcher - https://github.com/nodejs/undici#undicisetglobaldispatcherdispatcher

This allows you to Mock upstream HTTP servers for testing - here's an example

const mf = useMiniflare(
  {},
  {
    modules: true,
    script: `export default {
    async fetch(request) {          
      return await fetch(\`https://miniflare.dev\`);
    }
  }`,
  }
);

// Create a Mock Agent and prevent internet access - https://github.com/nodejs/undici/blob/main/docs/api/MockAgent.md
const agent = await mf.createMockAgent();
agent.disableNetConnect();

const client = agent.get("https://miniflare.dev");
client.intercept({ path: "/", method: "GET" }).reply(200, "Hello World!");

const dispatcher = await mf.getGlobalDispatcher();
await mf.setGlobalDispatcher(agent);

const res = await mf.dispatchFetch("http://localhost/");
// Check we get the mocked response and not the actual miniflare.dev
t.is(await res.text(), "Hello World!");

// Reset the Dispatcher for Other Tests - it would be nice if there was a slightly cleaner way of handling this
mf.setGlobalDispatcher(dispatcher);

There may be a more user friendly ways of handling mocking - having to reset the global dispatcher at the end of the test is a bit annoying but this is hopefully a good and usable start

Before submitting this PR I have

  • run prettier on all files
  • run all the unit tests
  • written some new unit tests to specifically test this functionality including a test that uses the miniflare jest environment

As mentioned there doesn't seam to be a formal contributors guide so if I've missed anything please let me know and I'm happy to amend

@william1616
Copy link
Contributor Author

@mrbbot just wondering if you've had a chance to look at this? happy to make any changes you see fit but mocking is a feature miniflare really needs!

@mrbbot
Copy link
Contributor

mrbbot commented Jul 4, 2022

Hey! 👋 Apolgoies for the delayed response, I wanted to discuss some things internally first. Thank you very much for submitting this comprehensive PR! 😃

Following the announcement that the Workers runtime will be open-sourced, Miniflare is going to be evolving significantly so that it can run your workers locally exactly as deployed.

As part of this evolution, it's very likely we won't be using undici as our fetch implementation, instead favouring the actual Workers runtime's.

This means I don't want to tie Miniflare's public-facing API to undici implementation details (i.e. the Dispatcher interface), as this would need to be removed in the next major version, leading to breaking changes... 🙁


However, undici's MockAgent API based on the popular nock package is really nice. Whatever fetch implementation we end up with, it should be possible to reimplement a similar API, leading to minimal breaking changes.

Another issue with this PR at the moment is that dispatchers are set globally. This means if you construct 2 new Miniflares, then call setGlobalDispatcher on each of them with different dispatchers, the last one will win and override the one set in the other Miniflare instance.


So I'd suggest the following changes:

  1. Create a function accepting no arguments createFetchMock that returns a MockAgent. This should be synchronous, and not part of MiniflareCore. This can't take any arguments as we don't want to tie ourselves to undici's API. I'd suggest putting it in packages/core/src/standards/http.ts.

  2. Add a new OptionType.NONE fetchMock option to CorePlugin here:

    @Option({ type: OptionType.NONE })
    logUnhandledRejections?: boolean;

  3. Since we need this in WebSocketPlugin too, add fetchMock to the PluginContext here:

    // Build compatibility manager, rebuild all plugins if reloadAll is set,
    // compatibility data, root path or any limits have changed
    const { compatibilityDate, compatibilityFlags, usageModel, globalAsyncIO } =
    options.CorePlugin;
    let ctxUpdate =
    (this.#previousRootPath && this.#previousRootPath !== rootPath) ||
    this.#previousUsageModel !== usageModel ||
    this.#previousGlobalAsyncIO !== globalAsyncIO ||
    reloadAll;
    this.#previousRootPath = rootPath;
    if (this.#compat) {
    if (this.#compat.update(compatibilityDate, compatibilityFlags)) {
    ctxUpdate = true;
    }
    } else {
    this.#compat = new Compatibility(compatibilityDate, compatibilityFlags);
    }
    const ctx: PluginContext = {
    log: this.#ctx.log,
    compat: this.#compat,
    rootPath,
    usageModel,
    globalAsyncIO,
    };

  4. Replace...

    getGlobalDispatcher(),

    ...with this instanceof Dispatcher ? this : getGlobalDispatcher() to allow a custom dispatcher to be passed via a bound this.

  5. Bind fetchMock to fetch's this if it's set in CorePlugin and WebSocketPlugin.

Replace...

fetch: createCompatFetch(ctx),

...with createCompatFetch(ctx, fetch.bind(this.fetchMock)).

Replace...

return fetch(request);

...with return fetch.call(this, request) to propagate this properly.

Replace...

this.#upgradingFetch = createCompatFetch(ctx, upgradingFetch);

...with createCompatFetch(ctx, upgradingFetch.bind(ctx.fetchMock));.

  1. For the Jest environment, I'd create a single MockAgent, set it as the fetchMock, then expose it globally via a getMiniflareFetchMock function or something.

Let me know if you have any questions. Feel free to DM me on Discord too. 🙂

@william1616 william1616 reopened this Aug 1, 2022
@william1616
Copy link
Contributor Author

@mrbbot thanks for the comments - I'm almost done addressing them; the one thing I can't get working is Jest

it looks like mf.setOptions isn't correctly setting the fetchMock - https://github.com/cloudflare/miniflare/pull/293/files#diff-030a0f6278d54bf8f41b941035a028c124f4b996e01f97b995fda35425cc668aR233 however I can't see why; mf.setOptions() will call #init() which should set fetchMock into the PluginContext (to test the context is actually being updated i've tried forcing ctxUpdate to true which doesn't seam to work) - hopefully you'll have an idea

once that's sorted it should be good to go

Copy link
Contributor

@mrbbot mrbbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! 👋 Apologies for the delayed response. I've recently returned from a long holiday and am catching up on issues now. Thanks for making those changes. 🙂 I've added some suggestions.

packages/core/test/index.spec.ts Outdated Show resolved Hide resolved
packages/jest-environment-miniflare/src/index.ts Outdated Show resolved Hide resolved
packages/jest-environment-miniflare/src/index.ts Outdated Show resolved Hide resolved
@william1616
Copy link
Contributor Author

@mrbbot all changes made - ready for a re-review!

Copy link
Contributor

@mrbbot mrbbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! 😃 Thank you! I've just pushed a couple minor linting fixes, but LGTM! ✅

@mrbbot mrbbot merged commit 3e459b7 into cloudflare:master Aug 18, 2022
@william1616
Copy link
Contributor Author

Great! Would love to start using this ASAP - does miniflare work to a planned release schedule or are new versions pushed more ad hoc?

@mrbbot
Copy link
Contributor

mrbbot commented Aug 18, 2022

Ad hoc releases, but I'd like to get one out in the next few days. 🙂

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

Successfully merging this pull request may close these issues.

3 participants