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

Updated to also use query string to match registered interactions. #31

Closed
wants to merge 1 commit into from

Conversation

hiroyoneyama
Copy link

We had multiple end points that we wanted to test and the path were the same and query string were different. I've made an update to match registered interactions if the query string also matches.

@neilcampbell
Copy link
Member

Hi @hiroyoneyama,
Thanks for sending through the PR. It has actually sparked a bit more conversation with @bethesque and looks like we will need to also compare the rest of the request, as that was the intention of the spec, however I misinterpreted it. We have updated the spec to include the changes based on our discussion. I will get started on a fix for this soon (hopefully this week or next). I have raised an issue, so you can keep track. #32.

@bethesque
Copy link
Member

The PR is actually one step closer to what we're aiming for, so maybe it's worth merging it?

@neilcampbell
Copy link
Member

@bethesque It is, however it is a direct string match on the query and path. So will not be a completely accurate match as we now parse the query as a NameValueCollection (in a less strict way).

@bethesque
Copy link
Member

Ah, right, I see what you mean.

On Tue, Oct 28, 2014 at 9:51 AM, Neil Campbell notifications@github.com
wrote:

@bethesque https://github.com/bethesque It is, however it is a direct
string match on the query and path. So will not be a completely accurate
match as we now parse the query as a NameValueCollection (in a less strict
way).


Reply to this email directly or view it on GitHub
#31 (comment).

@hiroyoneyama
Copy link
Author

Ok thanks guys. The NameValueCollection makes sense.

@neilcampbell
Copy link
Member

Hi @hiroyoneyama
Sorry this one has taken a little while to get actioned, however it is now fixed in 9bf09af and released in 0.1.5-beta.

Can you please test the latest nuget package and double check that this solves your issues?

Cheers,
Neil

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

Successfully merging this pull request may close these issues.

None yet

3 participants