Skip to content
This repository has been archived by the owner on Dec 10, 2022. It is now read-only.

Allow multiple requests on one route #14

Closed
wants to merge 2 commits into from
Closed

Conversation

robertkowalski
Copy link
Contributor

Some tests need multiple requests and using plugins is not that
handy if you want to use preconfigured routes.

If you do not provide an explicit option for that, it will
set the maxRequests to 1 - which is the current default.

Additionally, options are now passed 1:1 to hock, if they are an
object

@robertkowalski
Copy link
Contributor Author

review? @domenic?

@domenic
Copy link
Contributor

domenic commented Feb 24, 2014

Why would this not always be set to Infinity? Are there parts of the non-mock npm registry that only allow you to access them once?

@robertkowalski
Copy link
Contributor Author

It was limited to one request before, when it was not using many - I did not want to introduce a breaking change for npm-registry-mock.

https://github.com/mmalecki/hock/blob/master/lib/request.js#L43

But maybe we should set the limit to Infinity, and if people need smaller limit, they can set it with the options. I would raise the version to 1.0.0 then, what do you think?

@domenic
Copy link
Contributor

domenic commented Feb 24, 2014

That would make sense to me, but you know more about what would be more useful for npm-registry-mock in relation to the npm tests, so I am not sure.

Some tests need multiple requests and using plugins is not that
handy if you want to use preconfigured routes.

If you do not provide an explicit option for that, it will
set the maxRequests to Infinity. Before there was a `many`
method in hock it had a hard limit of 1.

Additionally, options are now passed 1:1 to hock, if they are an
object
@robertkowalski
Copy link
Contributor Author

I slept a night over this:

It is more convenient and helpful, also but not only for first-time-users, to allow infinite requests.

Even after writing alot of tests I never had the problem of an unknown, additional requests that was caught by the limit. But I often had just problems with the limit as I needed two requests for a package.

@robertkowalski
Copy link
Contributor Author

@domenic updated!

@domenic
Copy link
Contributor

domenic commented Feb 26, 2014

LGTM aside from documentation nit. Fix that and merge it :)

@robertkowalski
Copy link
Contributor Author

merged as 5f63af0 and 4f32ee1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants