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

Bugfix Interceptor.filteringPath #1543

Merged
merged 2 commits into from May 7, 2019

Conversation

mastermatt
Copy link
Member

Calling filteringPath on the intercept instance was broken as the
transform fn set on the scope had the wrong name.
Proxying to the Scope’s method allows for the regex version to work too.

The bulk of the changed lines come from moving the tests to their
appropriate file since the real logic acts on the Scope.

Found when looking at uncovered lines in Coveralls.

Side work, pulled out from #1520.

Calling `filteringPath` on the intercept instance was broken as the
transform fn set on the scope had the wrong name.
Proxying to the Scope’s method allows for the regex version to work too.
The bulk of the changed lines come from moving the tests to their
appropriate file since the real logic acts on the Scope.

Found when looking at Uncovered lines in coveralls.
@paulmelnikow paulmelnikow force-pushed the bug/interceptor-filtering-path branch from 6188824 to ee775b8 Compare May 7, 2019 13:20
Copy link
Member

@paulmelnikow paulmelnikow left a comment

Choose a reason for hiding this comment

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

Thanks so much for breaking this out!


const { statusCode } = await got('http://example.test/', {
query: { a: '1', b: '2' },
})
Copy link
Member

Choose a reason for hiding this comment

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

These are fine for now (and I know you didn't write them, so this is just a thought for the future!

It's hard to tell from these tests what exactly the feature is for. They don't have descriptive titles, and as far as I can tell they are all positive tests – there's nothing that checks what happens when the request doesn't match the filter.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had the same thought, but didn't want to blow up the scope of this PR.

.get('/?a=2&b=1')
.filteringPath(() => '/?a=2&b=1')
Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure I'm understanding: this test is of .filteringPath() called on an Interceptor. There's an identical "filter path with function" test of when .filteringPath() is called on a scope, that's being copied to test_scope. Is that right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct.

tests/test_intercept.js Outdated Show resolved Hide resolved
Co-Authored-By: mastermatt <matt@mattw.co>
@paulmelnikow paulmelnikow merged commit 560a5d8 into nock:beta May 7, 2019
@paulmelnikow
Copy link
Member

Awesome, thank you! 🙌

@nockbot
Copy link
Collaborator

nockbot commented May 7, 2019

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

The release is available on:

Your semantic-release bot 📦🚀

@mastermatt mastermatt deleted the bug/interceptor-filtering-path branch May 7, 2019 14:15
@nockbot
Copy link
Collaborator

nockbot commented Aug 12, 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
Calling `filteringPath` on the intercept instance was broken as the transform fn set on the scope had the wrong name. Proxying to the Scope’s method allows for the regex version to work too. The bulk of the changed lines come from moving the tests to their appropriate file since the real logic acts on the Scope.

Found when looking at Uncovered lines in coveralls.
gr2m pushed a commit that referenced this pull request Sep 4, 2019
Calling `filteringPath` on the intercept instance was broken as the transform fn set on the scope had the wrong name. Proxying to the Scope’s method allows for the regex version to work too. The bulk of the changed lines come from moving the tests to their appropriate file since the real logic acts on the Scope.

Found when looking at Uncovered lines in coveralls.
gr2m pushed a commit that referenced this pull request Sep 5, 2019
Calling `filteringPath` on the intercept instance was broken as the transform fn set on the scope had the wrong name. Proxying to the Scope’s method allows for the regex version to work too. The bulk of the changed lines come from moving the tests to their appropriate file since the real logic acts on the Scope.

Found when looking at Uncovered lines in coveralls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants