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

replace vitest-fetch-mock with msw #1592

Merged
merged 12 commits into from
Mar 29, 2024

Conversation

vipentti
Copy link
Contributor

@vipentti vipentti commented Mar 17, 2024

Changes

  • replaces vitest-fetch-mock with msw in openapi-fetch tests (including index.bench.js)

Fixes #1581

How to Review

I've made an effort to keep the existing tests as close as possible to their original implementation, with the exception of replacing mockFetch* with the equivalent msw test handler. Due to msw, it's necessary to pass baseUrl around to ensure we're using the correct request URLs. Therefore, it has been provided for each createClient and equivalent test handler.

Please note, the current usage of msw contradicts msw's best practices (Avoid Request Assertions). However, given the nature of this library, I believe this approach is acceptable in this context. In most of these tests, we aim to validate that the client is indeed calling the expected handler with the expected request parameters.

Checklist

  • Unit tests updated
  • docs/ updated (if necessary)

Copy link

changeset-bot bot commented Mar 17, 2024

⚠️ No Changeset found

Latest commit: 434afdf

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

@drwpow
Copy link
Contributor

drwpow commented Mar 17, 2024

This is a great PR, thank you! Very in favor of this direction—I had been wanting to do this but you beat me to it.

I’m not so much particular about the test setup details, so long as the tests themselves don’t change.

@vipentti vipentti force-pushed the feature/vitest-fetch-mock-to-msw branch from 0445fe4 to e07f193 Compare March 29, 2024 07:37
@vipentti vipentti marked this pull request as ready for review March 29, 2024 07:57
@vipentti
Copy link
Contributor Author

I don't have permissions to merge this, feel free to do so :)

@drwpow drwpow merged commit e89573e into openapi-ts:main Mar 29, 2024
6 checks passed
@drwpow
Copy link
Contributor

drwpow commented Mar 29, 2024

This is great, thanks again!

@vipentti vipentti deleted the feature/vitest-fetch-mock-to-msw branch March 29, 2024 14:22
@jshike
Copy link

jshike commented Apr 1, 2024

got an ETA for this to be published?

@drwpow
Copy link
Contributor

drwpow commented Apr 1, 2024

@jshike this is for testing; there’s nothing to publish

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.

Replace vitest-fetch-mock with msw in tests
3 participants