A few fixes that improve APIs used by VCR #154

merged 5 commits into from Feb 4, 2012


None yet

2 participants


Anyone want to code review this for me?

myronmarston added some commits Jan 28, 2012
@myronmarston myronmarston Fix #globally_stub_request to support multiple global stubs.
Previously it didn't fully support this (but didn't prevent it either) and it had weird ambiguities.  This cleans the code up a bit, too.
@myronmarston myronmarston Ensure that global stubs take precedence over non-global stubs.
This is important for tools like VCR that use global stubs for more than just returning a stubbed response--for consistency, it needs to always have the stub block invoked so it can manage before/after request hooks.
@myronmarston myronmarston Use the same request signature object in multiple hooks for the same

Previously, the httpclient and typhoeus adapters would use different
request_signature instances for the global_stub hook and the
after_requets hook. Using the same instance makes it more consistent and
enables a new VCR feature.
@myronmarston myronmarston Fix RequestSignature#==.
Previously it just used the definition in Object#== which only returned true for the same object instance.
@myronmarston myronmarston Fix spec failing on 1.8.7.
Weirdly, it appears that on 1.8.7, Array#delete will return the object you pass to it rather than the object in the array!

O = Struct.new(:a, :b)
i1, i2 = O.new(3, 5), O.new(3, 5)
list = [i1]
deleted = list.delete(i2)

deleted.equal?(i1) # => true on 1.9.2, false on 1.8.7
deleted.equal?(i2) # => false on 1.9.2, true on 1.8.7
bblimke commented Feb 4, 2012

Hi Myron,

Could you describe in short why multiple global stubs are needed. Implementation looks good.

Why is it important to return exactly the same RequestSignature object?
The change in Typhoeus looks good as an optimization :)

I don't understand the change in HTTPClient adapter. Was build_request_signature called twice for the same request signature anywhere?


I don't actually need multiple global stubs; it's more that the old implementation allowed multiple until WebMock.reset! was called, and then it kept just the last one. This was buggy behavior, IMHO. I also like that this change means that end users can use the global stub hook as well w/o interfering w/ VCR's hook. I'm ok rolling this change back but we should fix the bug in the old behavior regardless.

In VCR, a new feature requires that I preserve some state between the global stub hook (I.e before the request) and the after hook. The easiest way to do that is to assign an ivar on the request signature object in the before hook and then extract it in the after hook. Most of the adapters already worked this way; only Typhoeus and HTTPClient were broken.

When an async HTTPClient request was made it built the signature twice and used a different instance for the stub hook and the after hook. You can see this behavior if you check out this branch, revert the http client change and run the test I added for this.

@bblimke bblimke merged commit 1e658f8 into master Feb 4, 2012
bblimke commented Feb 4, 2012

Thanks. It's all clear now. I suspected there is something about async httpclient, but wasn't sure. It's merged into master now.

@davidbegin davidbegin deleted the vcr_fixes branch May 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment