I’m not sure about the naming of scope.assert(), it reads as if I should pass in something. Maybe scope.assertDone() and scope.assertAllDone()?

Hmm yea it does seem like it wants an argument. nock.assertAllDone() and scope.assertDone() could work. It's a little long but maybe that's fine.

For someone encountering the test code for the first time, it's definitely clearer than done().

I’m a fan of clear method names, even if they are long. Code readability is king 👑

Are there any other mock libraries with interfaces you like? I'm curious to compare. I looked at Sinon, but it doesn't really have assertion methods like this.

"Done" isn't the clearest either. Really what we want to say is "expectations fulfilled" or "requests made" or "mocks consumed." I'm 100% agreed that clear long names are better than short names, though I don't want the names to be longer than necessary.

Another option is nock.verifyAll() / scope.verify().

I don’t think it has the same problem as assert of wanting an argument.

Or: nock.verifyAllRequests() / scope.verifyRequest()

I changed nock.assertAll() to nock.assertAllMocksUsed() and scope.assert() to scope.assertMocksUsed().

now seeing that, I wonder why one of the method is called assertAllMocksUsed and the other assertMocksUsed. Wouldn’t it be more consistent to call both assertMocksUsed and the "scope" of the method call would be clear based on wether you call it on a scope instance or the global nock object?

It’s a good question. If the names are the same I think it’s too likely someone will call the wrong one. Since scope.done() was the way to do this for a long time, I don’t want to provide two different ways which are as confusable as scope.assertMocksUsed() and nock.assertMocksUsed(). Especially if those two functions are doing different things.

Another option: scope.assertUsed(), nock.assertAllMocksUsed().

Especially if those two functions are doing different things

Wouldn’t both functions do the same thing? nock.assertMocksUsed() would check globally, scope.assertMocksUsed() would check for only that scope?

I’d say let’s do it.

I wonder if it would be easier to move this out into its own RFC?

Awesome, let’s do it! No objections 👍

I feel either way could cause confusion. Having just one method name would make it easier to remember. But in the end I think it’s better than what we have right now and if you feel strong about nock.assertAllMocksUsed() being better than nock.assertMocksUsed() than let’s go ahead with it :)

If we have two different methods anyway and want them to look different, we could also do scope.assertScopeMocksUsed(). It's repetitive, but clear.

Agreed that assertMocksUsed() is better than assertAllMocksUsed(). 👍

Right, my concern is offering two identically named methods which act on different scopes. Most developers won't digest the whole API, they'll just absorb the bits they need. If they know the method is assertMocksUsed(), they could get unexpected results if they don't realize that calling it on scope will have a more limited effect than calling it on nock.

Having reflected on this a while, I'm okay with naming them both assertMocksUsed(). However I prefer your suggestion of nock.assertMocksUsed() for the global one and scope.assertScopeMocksUsed() for the limited one:

1. It encourages people to use the global nock.assertMocksUsed() where possible.
2. It makes the limited scope of scope.assertScopeMocksUsed() 100% clear.
3. It helps clarify that the intent of scope.assertScopeMocksUsed() is for cases which can't be handled using the global method.

If we do two different method names, then I’d prefer nock.assertAllMocksUsed over nock.assertMocksUsed. The latter might suggest I can pass an argument

Maybe we can call it .throwOnUnmockedRequests() might be more clear of what the method does?

On a big picture perspective, I feel like things like this should be an option when instantiating nock, which we currently don’t do, but I really think we should. We can discuss that in another RFC :D

maybe we could add two options:

• hosts: for a block list
• excludeHost: for an unblock list

Also I wonder if we could come up with a simpler alternative to Unreachability, it’s quite hard to spell :D

maybe .simulateConnectionError()?

I wonder whether this should be available on a scope? So for example

nock('https://google.com')
	.get('/')
	.simulateConnectionError();

Which would allow for some more granular control over which requests produce a network error and which don’t

That's an interesting idea, though there would also need to be a way to configure unreachability for unmocked requests.

I think .replyWithError() could be used for this purpose today, and would want to think about whether it's worth adding a second way to do it.

Yeah, I think it’s totally reasonable to also want to turn off all network connections as well. But yeah, I think it’s nice to be able to test how your application responds to one API being down/unavailable without it being blanketly based on the host.

The use case here is that I have two requests that go through an API gateway, in my case api.ft.com, behind that are different services which potentially could be individually unavailable. So api.ft.com/foo is available but api.ft.com/bar uses a different surface under the hood which is down.

Currently (as far as I know) I could only simulate all of api.ft.com being unavailable rather than just api.ft.com/bar.

Makes sense. Although, doesn't .replyWithError() solve that problem?

Yeah, I didn’t actually know that existed until you’d just mentioned it.

It does, however it might be nice to be able to not have to provide a message and have it just fail in the network and let whatever library (eg node-fetch) build the correct network error message, that it would if it was real, if that makes sense?

Right, makes sense. It's syntactic sugar, though it keeps the dev from needing to know what kind of error is thrown when a server is unreachable.

Yeah, exactly!

Agreed. I feel we can come up with a better way here

whenUnmocked feels like it should be an event handler somehow ... 🤔 I’m not sure about this one, I feel there is a better way. Sorry I know that’s not helpful but all I got right now 🤷‍♂️