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

feat(match_body): urlencoded body matches not only string values #1110

Merged

Conversation

arusakov
Copy link
Contributor

@arusakov arusakov commented Apr 5, 2018

It's first step to resolving issue #1106.

I think, that this behaviour is good, but can be unexpected for long-time nock users. Maybe we need an option to enable this behaviour?
Also RegExp values are turning into strings. It's not good. (Updated)
I can continue what I started if the whole approach is acceptable.
Review please @gr2m.

@gr2m
Copy link
Member

gr2m commented Apr 6, 2018

hey sorry I’m out vacationing (mostly) until April 22nd. At first glance the changes look good to me, as long as all current tests pass I’m okay with it. If we break things, we’ll fix them and improve the tests :)

@arusakov
Copy link
Contributor Author

arusakov commented Apr 8, 2018

Little update with one more test. RegExp values are processed correctly.

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.

Thanks @arusakov, looking great!

@nockbot
Copy link
Collaborator

nockbot commented Apr 9, 2018

🎉 This PR is included in version 9.2.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@arusakov arusakov deleted the 1106-urlencoded-body-matches-non-string-values branch April 10, 2018 08:00
@leggsimon
Copy link

Just an FYI, it seems that this broke our tests, since we needed to JSON.stringify some bodies instead now. Not sure this should have been a patch release

@arusakov
Copy link
Contributor Author

@leggsimon
Thank you for feedback. Can you provide some test cases?

@leggsimon
Copy link

@arusakov I'll try and get around to it today yes

@lock
Copy link

lock bot commented Sep 13, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue and add a reference to this one if it’s related. Thank you!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 13, 2018
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

4 participants