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
Bugfix Interceptor.filteringPath #1543
Conversation
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.
6188824
to
ee775b8
Compare
There was a problem hiding this 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' }, | ||
}) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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') |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.
Co-Authored-By: mastermatt <matt@mattw.co>
Awesome, thank you! 🙌 |
🎉 This PR is included in version 11.0.0-beta.12 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 11.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
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.
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.
Calling
filteringPath
on the intercept instance was broken as thetransform 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.