Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

em-http-request problems with queries #210

Merged
merged 1 commit into from

2 participants

@wrozka

Hi,
I'm using webmock to test integration with pusher (vcr, webmock, pusher, pusher-client and em-http-request) and I have noticed that webmock was breaking the em-http-requests to pusher servers.

It modified the internal request/uri from em-http request. I fixed that by duplicating the uri for purpose of creating WebMock::RequestSignature.

Sorry for specles request, but I have no time for digging more in WebMock, believe me, it is covered in my project's test suite. ;)

@bblimke
Owner

Hi Pawel.

Thank you for this pull request and fixing the problem.

I do believe this fixes the problem in your project and that it's covered in your project but I'm afraid I will need a spec in WebMock anyway.

The spec is needed for two reasons:

1) It shows why this code was added in git history if someone plans to change this code in the future.
2) It protects against regressions in WebMock if someone breaks this functionality i.e. in some
future pull request.

If not the spec, are you able at least to show a sample code that is broken without your changes?
I need to know what's exactly being fixed before merging this code :)

Have a look at this pull request. Is it something similar?
6927943

@wrozka

Hi Bartosz,
I agree in 100% that test is needed, I simply had no time to add it.
That is the same problem as in the @jonleighton's pull request, but in the 0.x version. He added the spec and there is only one for both em-http-request versions so bundling with old em-http-request should reproduce the problem.

@bblimke
Owner

Thanks for checking that. I'll ensure this spec runs for both adapters and merge your code when I have a minute.
Cheers!

@bblimke bblimke merged commit b7ebbab into bblimke:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 6, 2012
  1. Fixes problem with failing em-http-request with queries

    Pawel Pierzchala authored
This page is out of date. Refresh to see the latest.
View
4 lib/webmock/http_lib_adapters/em_http_request/em_http_request_0_x.rb
@@ -90,11 +90,11 @@ def build_request_signature
if @req
options = @req.options
method = @req.method
- uri = @req.uri
+ uri = @req.uri.dup
else
options = @options
method = @method
- uri = @uri
+ uri = @uri.dup
end
if options[:authorization] || options['authorization']
Something went wrong with that request. Please try again.