Port wildcard support for nockBack mocks

[kibertoad]

Context

Currently you need to explicitly manage port on which your test application is running when executing nockBack mocks so that recorded port matches the one you are currently running application on. This is a major inconvenience since supertest randomizes execution port.

It should be possible to specify wildcard port (*) that would match request against any possible port value or possibly its absence. Alternatively a parameter that would accept any port or its absence when no port is specified could also be a solution.

Alternatives

It is possible to manually launch and stop server for application under test that would be listening to specific port, but starting and stopping server manually is both introducing additional boilerplate and is not even that easy when you are executing multiple tests in parallel (as then you have to solve port collision problem yourself).

If the feature request is accepted, would you be willing to submit a PR?

Yes

[mastermatt]

Implementation-wise this feature revolves less around nockBack and more around how the core Scopes manage ports.
Currently, when an non-regex is passed to a Scope, it will parse the URL and if a port is missing, it will default to 80 or 443 based on the protocol.

nock/lib/scope.js

Line 58 in edce256

this.urlParts.port || (this.urlParts.protocol === 'http:' ? 80 : 443)

Unfortunately, ports are never directly compared when a request is being compared for a match. They are concatenated into a "base path" string that is then compared against with other whole strings or regex.
So there are some non-trivial, underlying implementation details that would need to be addressed before this feature could be rolled out.

This first question to ask is probably: what should the user interface be for this feature? I can think of a couple options:

1. First, this is technically already possible if a regex is provided to a Scope, but you can't define regex in JSON, so we could add an option to a Nock Definition that the "scope" value should be passed to a RegExp constructor before being passed to a Scope constructor.
2. The options passed to a new Scope could get some new bool option to ignore port during matching. All "extra" options on a Nock Definition already get passed to the Scope constructor so the nockBack aspect would just work.
3. Scopes could accept base paths with an asterisk port, indicating that the port should not be matched against. eg nock("https://example.com:*"). This might be the most intuitive option for users. Although, it won't play well with url.parse nor new url.URL so the asterisk would have to be sniffed out and removed before the rest of the base path was parsed.
4. Scopes could stop defaulting to a port at all. This would be a breaking change. It would mean that, by default, a setup like nock("https://example.com") would match against any port and if the user wanted to enforce a "standard" port, they'd have to provide it, eg nock("https://example.com:443").

I'll leave this comment as an open question.
The first option is the only one that wouldn't require major work in Scope matching. Although I think some of that work would probably be a good thing, and I think some progress is already being made on that front.
The first option may also be a good feature by itself, even if we decide to move on one of the other options specifically for ports. It comes across as a shortcoming that Scopes accept a regex for the base path, but Definitions can't utilize that.

Personally, I'm on the fence. I think I dislike the asterisk the most, at that point just provide a regex. Adding an option to Scope feels kludgy, more like a bandaid than a solid design. As is my M.O., I think I'm in preference of the breaking change option. Not only for port, but also for protocol. If a user called nock("example.com"), it seems reasonable that it would match "http://example.com:80", "http://example.com:3000", "https://example.com:443", etc.

[paulmelnikow]

Is it possible that this use case could be handled using .filteringScope()?

Basically it lets you write a function to handle host matching. It's designed for wildcard hostnames, though I think it already works for ports.

I'm open to redesigning / improving the API including breaking changes and happy to discuss that as well. To be honest, I think .filteringScope() is a bit awkward (although it is powerful).

[mastermatt]

Is it currently possible to use filteringScope via Nock Back though?

[paulmelnikow]

Indeed not. That part would need to be added.

[simlu]

This would be a great feature

[simlu]

Anyone have a clue how one would go about adding a filteringScope on nockBack? I'm having a hard time with this code base 😬

[simlu]

ping

[kibertoad]

@simlu This is the solution that we ended up building in adidas for this problem: https://www.npmjs.com/package/nockback-harder

[gr2m]

@kibertoad would you be interested in contributing it back to nock? I'll work on moving the recording logic into its own @nock/recorder (or similar) module over the next weeks and would happily add you as maintainer. The current problem with the recording/Nock Back features is that none of the current maintainers use it

[kibertoad]

@gr2m I would be happy to. Please ping me when there will be something I could be moving it to.

[gr2m]

Will do

[kaminskypavel]

ummm.. is this issue dead? 🤔

[simlu]

The problem is that the code base is really hard to work with. So I couldn't figure out where the appropriate place to add this feature was

But the request is very much still relevant