Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Matching partial body params #77

Closed
wants to merge 1 commit into from

4 participants

@karmajunkie

The current implementation of matching the body as a hash seems flawed in that it only matches when the pattern contains the entirety of the body hash. For example:

#connection class
post(SERVICE_URI, :body => {:username => "testname", :password => "testpassword"})

#spec
WebMock.should have_requested(:post, SERVICE_URI).with(:body => {"password" => "testpassword"})

The spec in this case fails in the current implementation, as it doesn't completely specify the hash that was passed into the post. I can't think of any situations where my proposed method of specifying partial matches should reasonably fail—and if that is the desired functionality, it would seem that one should specify that instead of letting a magic default in a mocking library allow your test to pass. Using this patch the above specification would indeed pass.

@jcf
Collaborator
jcf commented

Does the following not work?

WebMock.should have_requested(:post, SERVICE_URI).with do |request|
  request.body['password'] ==  'testpassword'
end

Is there a particular reason why the block approach is less preferable to changing #with?

@jcf jcf closed this
@marnen

No, indeed, @jcf's solution doesn't work. request.body in the block is a string, not a hash, so while it responds to [], it does so with completely wrong results. I support changing with as requested.

@marnen marnen referenced this pull request
Closed

Please reopen #77 #142

@bblimke bblimke reopened this
@bblimke
Owner

@jcf's previous example doesn't work indeed. The following would work though:

WebMock.should have_requested(:post, SERVICE_URI).with do |request|
Crack::JSON.parse(request.body)['password'] == 'testpassword'
end

I understand it's not convenient and I'd be happy to find some simpler solution.

Current behaviour of #with can't be changed, at least not in versions 1.x. This change would be backward incompatible
and in many cases matching exact body params is desired. That's why this pull request can't be accepted.

WebMock would need something like #hash_including arguments matcher from Rspec.

Any ideas on how webmock's api could be changed to support it?

@marnen

I've been doing something much like your example, but actually using CGI.parse (this is just standard postdata, not JSON).

As for syntax...perhaps borrow RSpec's API: .with(:body => hash_including(:some => 'parameters'))?

@bblimke
Owner

I thought about it, but hash_including is already reserved by RSpec (returns rspec matcher). In webmock it will have to return some webmock specific matcher, that can be used independently from test framework.

@marnen

Hmmm. We can't reuse RSpec's matcher -- test that the type of body is a HashIncluding (or whatever the class is), and if so, run the parsed params through RSpec? (I'll try to play with the code if I have time...I'm very busy this week, though.)

@bblimke
Owner

Possibly. Webmock would use hash_including from rspec if available and if not it would provide own hash_including.

@marnen

That sounds reasonable.

@bblimke
Owner

hash_including support is in master. It works for :body and :query options.
It uses RSpec's hash_including if available.

@bblimke bblimke closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 2, 2011
This page is out of date. Refresh to see the latest.
Showing with 11 additions and 2 deletions.
  1. +5 −1 lib/webmock/request_pattern.rb
  2. +6 −1 spec/request_pattern_spec.rb
View
6 lib/webmock/request_pattern.rb
@@ -147,7 +147,7 @@ def matches?(body, content_type = "")
when :xml then
Crack::XML.parse(body) == @pattern
else
- Addressable::URI.parse('?' + body).query_values == @pattern
+ hash_contains?(Addressable::URI.parse('?' + body).query_values, @pattern)
end
else
empty_string?(@pattern) && empty_string?(body) ||
@@ -161,6 +161,10 @@ def to_s
end
private
+
+ def hash_contains?(haystack, needle)
+ needle.select{|k, v| haystack[k] != v}.empty?
+ end
def empty_string?(string)
string.nil? || string == ""
View
7 spec/request_pattern_spec.rb
@@ -214,7 +214,12 @@ def match(request_signature)
should match(WebMock::RequestSignature.new(:post, "www.example.com", :body => 'a=1&c[d][]=e&b=five&c[d][]=f'))
end
- it "should not match when hash doesn't match url encoded body" do
+ it "should match if body is a superset of pattern hash" do
+ WebMock::RequestPattern.new(:post, "www.example.com", :body => {:a => "1"}).
+ should match(WebMock::RequestSignature.new(:post, "www.example.com", :body => 'a=1&c[d][]=e&c[d][]=f&b=five'))
+ end
+
+ it "should not match when hash is not completely contained by url encoded body" do
WebMock::RequestPattern.new(:post, 'www.example.com', :body => body_hash).
should_not match(WebMock::RequestSignature.new(:post, "www.example.com", :body => 'c[d][]=f&a=1&c[d][]=e'))
end
Something went wrong with that request. Please try again.