Use msw to test http-client requests

[diehuxx]

TL;DR I'm using msw to write tests, but I'm using msw weirdly, but I have good reasons.

Draft Phrase Preamble

Before I dive in and write ~20 tests using this approach, I want to get some soft consensus that people like testing the http-client this way. I tried several different libraries for mocking fetch and testing fetch requests, and honestly don't like any of them, but I've settled on an approach that works for our use case.

Overview

Our current tests in the http-client are brittle. We test that we can call methods like TbdexHttpClient.sendMessage to make sure that they do not throw, but we have no assertions that we are actually sending a message to the correct url with the correct request structure. The aim of this PR is to add those tests.

We select msw to accomplish this because msw has allows us to record requests to an endpoint and msw supports a wider variety of environments that other libraries.

Testing libraries comparison

I'll start by saying I settled on msw, but deliberately going against some of its recommended best practices. More detail below.

nock

I found nock the most ergonomic for setting up one-time request interceptors. Its API was exactly what I wanted. Sadly it has limited environment support, and I don't want to have to re-write all these tests again in a month when nock inevitably doesn't cut it.

Polly.js

I had high hopes when I saw Netflix built it, but the docs are all over the place and it's pretty complex to set up. The quickstart is broken and uses deprecated APIs, and finally saw that the adapter I needed was built on top of nock anyway.

msw

Rant: I tried msw first, hated it, then came crawling back. I don't like that msw feels like reimplementing the server in the test files. I REALLY don't like having a separate handlers.js where all the default mocks request handlers go. Kent C Dodds seems to love how short it makes his tests, but personally I find it hard to read what the test is actually testing for.

To better accommodate my neuroses our use case, these tests deliberately go against some of MSW's officially recommended best practices. In particular, these tests make request assertions . MSW's reasoning is that request assertions tend to test implementation rather than behavior. This makes sense for apps where one entity controls (and can freely make changes to) both the client and server. However, we are implementing a spec, and must be sure that our client adheres precisely to the spec in the requests' url, header, and body.

Though other libraries (e.g. nock) supply assertions for request url, header, and body matching out of the box, we choose MSW because it has support for a wide variety of environments. To adapt it for our use case, we make extensive use of server.use(http.get({ once: true, ... })) as a form of assertion.

[changeset-bot]

⚠️ No Changeset found

Latest commit: fa0149e

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

[github-actions]

TBDocs Report

✅ No errors or warnings

@tbdex/protocol

• Project entry file: packages/protocol/src/main.ts

@tbdex/http-client

• Project entry file: packages/http-client/src/main.ts

@tbdex/http-server

• Project entry file: packages/http-server/src/main.ts

---

TBDocs Report Updated at 2024-02-29T19:10:35Z fa0149e

[diehuxx]

Temporarily commenting this file out because it includes stubs that bleed over into other tests.

[diehuxx]

Created issue to track this #197

[diehuxx]

Closing since I don't have time to get this ready for review, and we have an issue to track it. #197