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

Fix incorrect request body when intercepting fetch with getMiniflareFetchMock #423

Merged

Conversation

robertcepa
Copy link
Member

@robertcepa robertcepa commented Nov 6, 2022

Hey there!

I am using miniflare with jest to test a Workers library. I need to write a simple unit test that intercepts a request body and returns it in response.

Example (full code here):

fetchMock.get('...')
  .intercept({
    method: () => true,
    path: () => true,
  })
  .reply(200, (opts) => {
    return opts;
  });

The issue I'm encountering is that opts.body doesn't contain request body string, as expected per https://undici.nodejs.org/#/docs/best-practices/mocking-request?id=reply-with-data-based-on-request , but AsyncGenerator.

I first thought that this was an issue in undici but after discussion with their team (nodejs/undici#1756) it seems that the solution is to expose isMockActive property on Dispatcher for test utilities on MockAgent to work properly.

This PR adds a new MiniflareDispatcherMock class that extends the base MiniflareDispatcher with isMockActive getter. I decided to not add the getter directly to MiniflareDispatcher, because it's a testing function that doesn't exist on real dispatchers.

This class is used on fetch polyfill to instantiate dispatcher when bound with MockAgent.

@changeset-bot
Copy link

changeset-bot bot commented Nov 6, 2022

⚠️ No Changeset found

Latest commit: f044199

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

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! 👋 Thanks for the PR! Added a small comment. 👍

packages/core/src/standards/http.ts Show resolved Hide resolved
@robertcepa robertcepa force-pushed the robertcepa/fix-request-body-in-reply branch from 7024e9a to f044199 Compare November 8, 2022 17:39
@mrbbot mrbbot merged commit 2e49dab into cloudflare:master Nov 9, 2022
@sjblurton
Copy link

Well done @robertcepa this is something we have been trying to work around here. Thank you for the PR.

Hey @mrbbot, we have been trying to get this working here since mid-last year and have just noticed someone has already fixed it. Do you have any idea when this will be released? Thank you.

@ashleyoldershaw
Copy link

Hi! I'm also invested in seeing this released :) do we have a timeline?

@sjblurton
Copy link

@mrbbot was talking about this PR today with the team. Is there a plan for a wide release? The automated testing we are trying to implement needs this.

@sjblurton
Copy link

Can we get any updates on this one, please? We really want to be able to test our API. 😁

@CameronB15
Copy link

Just stumbled across this PR when trying to figure out how to unit test my worker. Do you know when this will be added to a release @mrbbot?

@mrbbot
Copy link
Contributor

mrbbot commented Feb 14, 2023

Hey! Miniflare 2.12.0 has just been released with this change! 🙂 Apologies for the delay here.

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.

5 participants