Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Version 1.10 incompatible with earlier 1.x gems #259

Closed
sferik opened this Issue · 10 comments

3 participants

@sferik

A backward-incompatibility was introduced in this commit: fe1af0c. I added my 2 cents in a comment at the bottom, so I won't repeat it here.

@sferik

Here is an example where tests were passing on webmock 1.9.3 and started failing on 1.10.0: https://gist.github.com/sferik/84a9b94df1188b69b10f

@sferik

Here's the patch I applied to make these specs pass again: sferik/twitter@89bbc04. It didn't take me very long to fix the problem but I would have been much less surprised by the breakage if it had occurred in a 2.0.0 release instead of version 1.10.0.

@sferik sferik closed this
@bblimke
Owner

@sferik Thank you for these examples. I didn't plan to make any changes to post request body with this commit, just to the query params in urls, so maybe there is a new bug introduced. I'll investigate that. Cheers.

@bblimke bblimke reopened this
@sferik

@bblimke I believe this issue was causing by removing .gsub(/\+/, " ")) from line 69 of lib/webmock/util/query_mapper.rb.

@bblimke
Owner

Indeed. the same method is used to parse url encoded body.
It's now fixed in version 1.10.1.
Cheers!

@sferik

I reverted sferik/twitter@0ff9f48 but still had to apply sferik/twitter@d0b777e to get specs passing on the latest webmock (1.10.1).

Here are the four failures I was getting: https://gist.github.com/sferik/cea8f2fef6e90d9f7dad

It appears to be encoding spaces (%20) as plusses (%2B). I believe this is a consequence of switching from using CGI.escape to URI.escape in fe1af0c#L0R34.

@i0rek

When I run the specs on master I see the following failures: https://gist.github.com/i0rek/8e8141d7ca90249752c2. Related?
Stumbled over that while releasing Typhoeus.

@bblimke
Owner

@i0rek yes it's related, but that was actually outdated spec

@bblimke
Owner

@sferik ok, this change to + was wrong after all. could you please check if new 1.10.2 version is fine?

@bblimke
Owner

@sferik btw. you shouldn't need to make any changes to your specs.

@bblimke bblimke closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.