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

hash_including not working at all? Doesn't find any query #371

Open
Fire-Dragon-DoL opened this issue Feb 28, 2014 · 11 comments
Open

hash_including not working at all? Doesn't find any query #371

Fire-Dragon-DoL opened this issue Feb 28, 2014 · 11 comments

Comments

@Fire-Dragon-DoL
Copy link

I wrote a test after finding out that my stubbed requests were not called correctly, here is a test code:

require 'spec_helper'

describe WebMock, focus: true do
  let!(:dummy_url) { 'http://dummyurl.com' }

  it "receive a request when mocked with query param on a regex" do
    stub_request(
      :get,
      /#{ dummy_url }/
    ).with(
      query: hash_including({
        param1: 5
      })
    ).to_return(
      body: 'body 1'
    )

    expect(
      HTTParty.get(dummy_url, {
        query: {
          param1: 5,
          param2: 'random1'
        }
      }).body
    ).to eq 'body 1'
  end

  it "receive a request when mocked with query param without a regex" do
    stub_request(
      :get,
      dummy_url
    ).with(
      query: hash_including({
        param1: 5
      })
    ).to_return(
      body: 'body 1'
    )

    expect(
      HTTParty.get(dummy_url, {
        query: {
          param1: 5,
          param2: 'random1'
        }
      }).body
    ).to eq 'body 1'
  end

end

Both of them are not passing, they do not receive the request.
Notice that if I use do end block in with, I do manage to receive the request so it's definitely hash_including the issue, expecially because if I remove it, the query matching works fine (if I add param2: 'random1').

@bblimke
Copy link
Owner

bblimke commented Feb 28, 2014

Try it with keys being strings not symbols.

@Fire-Dragon-DoL
Copy link
Author

@bblimke I think I've found the issue and it's quite stupid:

  it "receive a request when mocked with query param", focus: true do

    stub_request(
      :get,
      dummy_url
    ).with(
      query: hash_including({
        param1: '5',
        param2: 'random2'
      })    
    ).to_return(
      body: 'body 1'
    )

    expect(
      HTTParty.get(dummy_url, {
        query: {
          param1: 5,
          param2: 'random1'
        }
      }).body
    ).to eq 'body 1'
  end

The keys are not required to be strings. But the values must be strings.
Is it a bug? I think webmock can autoconvert everything to string easily if it requires that to perform the matching.

@dzajic
Copy link

dzajic commented Jul 17, 2014

@Fire-Dragon-DoL Thanks for the tip, this issue was killing me today!

@Fire-Dragon-DoL
Copy link
Author

@dzajic Happy to help you ;)
I tried writing a patch but I must admit I have difficulties not knowing at all the code base. I would have done it through Hashie gem usually.

@bblimke
Copy link
Owner

bblimke commented Jul 17, 2014

Since query values can only be strings, it indeed makes sense to autoconvert values to strings for comparison.

@davidbegin
Copy link
Collaborator

@Fire-Dragon-DoL if you like I can try and help you with a fix. I'm new to codebase too, but 2 eyes might help.

@zdennis
Copy link
Contributor

zdennis commented Jul 15, 2015

I hit this today as well. @presidentJFK or @Fire-Dragon-DoL, have either of you started on a patch?

@davidbegin
Copy link
Collaborator

@Fire-Dragon-DoL @zdennis I have opened a Pull Request #517 trying to fix this problem. If you could take a look and see if it fixes your problem. Maybe try the branch against your failing tests.

@Fire-Dragon-DoL
Copy link
Author

@presidentJFK seems good, one point though, maybe you want to stringify hash keys too?

@davidbegin
Copy link
Collaborator

@Fire-Dragon-DoL WebMock already converts keys to strings, I added a spec exercising this functionality on #517

@nomasprime
Copy link

This was driving me nuts too, thanks @Fire-Dragon-DoL 🙌

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

No branches or pull requests

7 participants