Replace fetch-mock with Mock Service Workers

[david-crespo]

Summary

As we have seen over and over lately, fetch-mock is not very fun to use. It's noisy, confusing, and has a bad API. Mock Service Workers provide a different approach: instead of manually mocking individual requests, define an entire mock server for your app to call. This mock server can can run in Node for testing purposes, but it can also run in the browser for demo or development purposes because it's designed to run in a Service Worker.

A mock server is arguably more elaborate, and risks making your tests depend on mocked logic that doesn't match the real API, but fetch-mock has this problem too, with the additional downside that the potentially wrong logic is distributed across the entire body of tests, rather than collected in one spot where its overall correctness can be considered.

Kent C. Dodds's post Stop Mocking Fetch makes a longer argument for this approach.

Next steps

In this PR we are using a custom override helper to override the response for particular tests, which feels rather like mocking fetch responses. It's still better overall because we only need overrides for cases where we want something other than the default response. But I think the way this is really supposed to work is to put that logic into the mocked handler. I'll put some comments in the code explaining how I think this should be done in particular cases.

More broadly, right now we're returning a default mock response in every case, but I'd like to get fancier and have what's essentially a mock database that looks like this

const initDb = {
  orgs: [mock.org],
  projects: [mock.project],
  instances: [mock.instance],
  ...
}

const clone = (o) => JSON.parse(JSON.stringify(o))

function resetDb() {
  db = clone(initDb)
}

let db
resetDb()

And we would put resetDb in an afterEach. Then our create endpoints could actually insert things into the list, our list endpoints could return the list, get could actually 404 if the thing isn't there, etc. This feels a bit elaborate, but once you have the basics in place it might feel pretty good. MSW have a library called @mswjs/data that's meant to help with all this, so I'll experiment with that too.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/oxidecomputer/console-ui-storybook/E43vQxcWvsQV9WoMvMix9KjqmWZb
✅ Preview: https://console-ui-storybook-git-msw-oxidecomputer.vercel.app

[github-actions]

Preview will be deployed at https://console-git-msw.internal.oxide.computer

[david-crespo]

This is clean enough, but we could also hard code a special instance name like "dupe" in the handler and have it return this error when you post an instance with that name. Or probably even better, once we have a fake instances list, we can have it actually detect a duplicate name.

[just-be-dev]

😄

[david-crespo]

moving this before build permanently because the two builds take 30 seconds and basically never fail. might as well get test failures sooner

[david-crespo]

I kind of miss these tests, but I get that they're an implementation detail. The more correct MSW way of doing it is to have MSW actually add the posted thing to a mock database and then assert that it shows up in the UI.

[just-be-dev]

Yeah, this is where that data library would come in handy.

[just-be-dev]

I agree that testing the body of the post is testing implementation details. Having the data layer plugged in means we could test the results of the post which feels like a more valuable test to me. The important part here is that we don't actually want the test to have internal details about the shape of the form. If we assert against the shape of the body then we're tightly coupling the test to the forms internal representation which would be best avoided.

[david-crespo]

That’s a good point, and it’s even stronger when you consider more complex inputs than text fields. With a text field, the internal representation of the value is just the string you typed (usually). But with a multi-select the internal representation is more opaque.

[just-be-dev]

This is a solid start. Removing the fetch mocking code makes the tests feel a lot cleaner. Trying out the data lib might be a good next step to be able to verify form posting results.