Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw error when leading slash is not present in path #1391

Merged
merged 3 commits into from Jan 25, 2019
Merged

Throw error when leading slash is not present in path #1391

merged 3 commits into from Jan 25, 2019

Conversation

kevinnio
Copy link
Contributor

@kevinnio kevinnio commented Jan 22, 2019

Previously, whenever you intercept a path with no leading slash nock will not resolve the URL correctly. This fixes the issue by adding a leading slash if required while creating an Interceptor.

Given #1391 (comment), we decided to make nock raise an error whenever the user forgets to add a leading slash into the intercepted path, instead of adding it itself.

Fixes #1259.

@paulmelnikow
Copy link
Member

Hi! Thanks for this contribution :)

New work is being done in the beta branch. Could you rebase this?

lib/interceptor.js Outdated Show resolved Hide resolved
tests/test_intercept.js Outdated Show resolved Hide resolved
@kevinnio kevinnio changed the base branch from master to beta January 23, 2019 19:01
@kevinnio
Copy link
Contributor Author

Rebased onto beta branch.

Previously, whenever you intercept a path with no leading slash nock will
not resolve the URL correctly. This fixes the issue by adding a leading
slash if required while creating an Interceptor.

Check #1259 to know more.
lib/interceptor.js Outdated Show resolved Hide resolved
Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great now! Thanks for bearing with us Kevin 👍

Given #1391 (comment), we
decided to make nock raise an error whenever the user forgets to add a
leading slash into the intercepted path, instead of adding it itself.
@kevinnio kevinnio changed the title Add leading slash when is not present in path Throw error when leading slash is not present in path Jan 24, 2019
Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@gr2m gr2m merged commit 28b2d43 into nock:beta Jan 25, 2019
@nockbot
Copy link
Collaborator

nockbot commented Jan 25, 2019

🎉 This PR is included in version 11.0.0-beta.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@paulmelnikow
Copy link
Member

@gr2m I noticed this is listed as a big fix but I think it might be better to call it out as a new feature when 11 is released.

@gr2m
Copy link
Member

gr2m commented Jan 25, 2019

works for me, we can edit the release notes

@nockbot
Copy link
Collaborator

nockbot commented Aug 13, 2019

🎉 This PR is included in version 11.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

gr2m pushed a commit that referenced this pull request Sep 4, 2019
Given #1391 (comment), we
decided to make nock raise an error whenever the user forgets to add a
leading slash into the intercepted path, instead of adding it itself.
gr2m pushed a commit that referenced this pull request Sep 4, 2019
Given #1391 (comment), we
decided to make nock raise an error whenever the user forgets to add a
leading slash into the intercepted path, instead of adding it itself.
gr2m pushed a commit that referenced this pull request Sep 5, 2019
Given #1391 (comment), we
decided to make nock raise an error whenever the user forgets to add a
leading slash into the intercepted path, instead of adding it itself.
@uxmaster
Copy link

Nock should not throw when the path equals '', to enable testing paths without trailing slash, see #2042 (comment)

uxmaster added a commit to uxmaster/nock that referenced this pull request Jan 16, 2021
This artificial requirement was introduced by nock#1391.
uxmaster added a commit to uxmaster/nock that referenced this pull request Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants