Allow more customization of network mocking urls

danwenzel commented 5 years ago

nock has several customization options that would be nice to implement in ember-cli-fastboot-testing's network mocking. Just to mention a few:

Setting of the hostname - Currently, we are limited to using the same hostname as the test runner. Oftentimes we have a separate api host that we'd like to override.
Customizing the path - Currently, we can only use exact path matches - it would be great to be able to use RegEx or the includes option.
Customizing the query string - Similar to above, it would be really helpful to explicitly specify the query string, use regex, or use nock's .query(true) option to ignore the query string altogether. Otherwise, it's only a match if the query string is appended to the url with the exact full string.

We also have this issue: https://github.com/embermap/ember-cli-fastboot-testing/issues

It seems like we either need a way to use nock's syntax when creating these mocks, or expand the API to allow several different customization options. Maybe something like:

await mockServer.get('/api/posts/1', { <body>}, { <options> } )

(we'd have to move the status code into an option, and maybe deprecate the string as a 3rd param)

I'd be happy to work on a PR for this once we land on the approach.

ryanto commented 5 years ago

Nice, totally agree!

There's some work being done in https://github.com/embermap/ember-cli-fastboot-testing/pull/51 that exposes a better API for post requests. We can do something similar for gets as well.

I thinking we should have MockGet (similar to MockPost from the above PR) that exposes these options via an API. We can pick and choose the options from nock we want to support, I think hostname and query(true) make sense.

Do you have an example of how you'd like to use RegExp w/ includes? Same for anything RegExp related in the query string. I'd love to see how that could be used in a test.

danwenzel commented 5 years ago

Awesome that this is getting worked on, @ryanto!

hostname and query(true) are definitely the ones we'd use the most.

As far customizing the path, the best example I can think of is dynamic segments. So something like /users/:id/profile, etc.

And for the query string, maybe something where we're checking for a certain key, but don't care what the value is. Maybe: /rentals?token=...

Ideally, we'd have the ability to use the function syntax in the reply and access the original request for these dynamic scenarios. But that may be out of scope for this.

CvX commented 5 years ago

I've finally fixed the fastboot tests in my app, and the hostname configuration is definitely a must. I ended up simply adding it as a first argument to all mocking methods, just to keep the ball rolling (https://github.com/embermap/ember-cli-fastboot-testing/compare/master...CvX:hostname).

I could also use the "disabling real http requests" (with nock.disableNetConnect()), to make sure absolutely no actual requests are made behind my back 😉). And maybe nock.recorder.rec() to take a peek at the data returned by the backend.

This made me wonder if it wouldn't be better to expose the whole nock functionality, via comlink-like interface:

import { nock } from 'ember-cli-fastboot-testing/test-support';

test('…', async function() {
  await nock.disableNetConnect();
  await nock('https://api.example.com')
    .get('/posts)
    .reply(200, …);
});

This would allow to use every feature nock has to offer, without having to patch it in, one by one, to the ECFT. The downside is the tight coupling to nock, so e.g. if this addon was to replace the mocking library with some other package - that would be a breaking change for addon's users. I think that's a fair tradeoff though.

danwenzel commented 5 years ago

Having direct access to the nock object would be awesome. @ryanto - thoughts? would that even be possible?

ryanto commented 5 years ago

Hey! Sorry for the late reply.

One of the reasons I didn't give direct access to the nock object is because the tests are written in the browser, but nock runs in a node process. I was unsure of how to serialize all those object and function calls across the browser/server boundary. That's the short of why the mocking API is so limited.

@CvX it sounds like comlink will give us a nice API for crossing that boundary? If so, I'm all for this! Giving full access to nock seems like it will solve a whole lot of these recent issues, and I'm not so worried about the tight coupling at this point since I don't really see the need for us needing to support multiple node mocking libraries.

Thoughts?

CvX commented 5 years ago

Alright, I initially thought I can get a proof-of-concept done in a single evening, but it took me a bit over two days. 😅

Comlink, in its current state, doesn't support the builder pattern (e.g. await thing().otherThing().somethingElse();). I'm going to submit a PR to comlink with my changes, after I clean up the code.

In the mean time, I have some other minor changes to ecft. I'll also start the PR with this new mocking interface to get things rolling.

ryanto commented 5 years ago

Wow, awesome!

CvX commented 5 years ago

Comlink PR is ready: https://github.com/GoogleChromeLabs/comlink/pull/311

And here's a preview of ember-cli-fastboot-testing changes: https://github.com/embermap/ember-cli-fastboot-testing/compare/master...CvX:nock-proxy

embermap / ember-cli-fastboot-testing

Allow more customization of network mocking urls #53