koala icon indicating copy to clipboard operation
koala copied to clipboard

Migrate away from supertest

Open nickmccurdy opened this issue 6 years ago • 3 comments

Now that we're using Jest, we can use Promise returns, async/await, and the .resolves and .rejects matchers instead of using the expectations coupled with supertest (which I have a feeling don't fully support Jest's logging the way that the built in matchers do).

Node HTTP clients using Promises

  • util.promisify(http.request) (Node 8 or the util.promisify shim package)
  • node-fetch (implementation of fetch for Node)
  • axios
  • r2
  • request-promise

nickmccurdy avatar Sep 07 '17 17:09 nickmccurdy

I tried to start this in #55, turns out it's a lot of work because we're coupled with how supertest can make requests against servers and callbacks. While Jest makes async expectations easy, we'd have to refactor all the tests to manually open and close servers or use a custom request utility that does this. The latter approach seems to be working so far but I'm going to take a break from this since it involves so much work migrating tests.

This may or may not be worth it. I don't like coupling the way we make requests to the way we make expectations about them because it makes it difficult to use Jest's awesome async matchers. But it may be more work than it's worth for now.

nickmccurdy avatar Sep 09 '17 14:09 nickmccurdy

btw supertest returns promises now

jonathanong avatar Sep 30 '17 19:09 jonathanong

Thanks, I'm aware of that, I actually used it in some in progress code for migrating away from supertest. However, Jest has resolves and rejects matchers and async/await support (and Babel support if you're testing on a Node version that doesn't have it natively). I'd rather stick to Jest's built in matchers for their improved error messages and framework integrations. We could also just switch from supertest to superagent, though we unfortunately lose the ability to pass a server to the initial request function.

nickmccurdy avatar Sep 30 '17 20:09 nickmccurdy