Created
November 9, 2014 16:20
-
-
Save jbrains/d6ba461beee0c4f01343 to your computer and use it in GitHub Desktop.
Is there any harm in using instance variables inside an RSpec Matcher like this? Will I keep garbage values from use to use?
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RSpec::Matchers.define :eventually_redirect_to do | target_url | | |
def collect_redirect_urls_from(url) | |
redirect_urls = [] | |
response = RestClient.get(url) do | response, request, result, &block | | |
redirect_urls.push(response.headers[:location]) if response.code == 301 | |
response.return!(request, result, &block) | |
end | |
redirect_urls | |
end | |
@redirected_urls = [] | |
match do | request_url | | |
@redirected_urls = collect_redirect_urls_from(request_url) | |
@redirected_urls.include? target_url | |
end | |
failure_message do | actual | | |
where_it_redirected_clause = @redirected_urls.empty? ? "redirected nowhere" : "only redirected to #{@redirected_urls}" | |
"expected response to redirect eventually to #{target_url}, but #{where_it_redirected_clause}" | |
end | |
failure_message_when_negated do | actual | | |
"expected response never to redirect eventually to #{target_url}" | |
end | |
describe do | |
"checks eventual redirect response" | |
end | |
end | |
What version of RSpec are you using? I ask because in your example, describe
should be description
. In older rspec 2 versions, describe
was unfortunately monkey patched onto every object, which meant that it was available in any context such as this. On RSpec 3 (and probably later RSpec 2 versions, although I didn't test this just now) you'll get a NameError
since we've been so much more intentional about eliminating monkey patching.
So the fact that you are able to use describe
like that suggests you are on an older RSpec 2 version, which may still have the instance variable bug you referenced. I'd highly recommend upgrading to RSpec 3 as there are so many bug fixes, new features and other improvements.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
That bug was fixed a long time ago. It's totally fine to use instance variables inside a DSL-defined custom matcher. In fact,
chain
almost always uses instance variables:http://www.rubydoc.info/gems/rspec-expectations/RSpec/Matchers/DSL/Macros#chain-instance_method
Note, however, that the
@redirected_urls = []
line isn't doing what you think it is -- that's because thedefine
block is essentially evaluated as a class body, while the individualmatch
, etc blocks define instance methods and will run with aself
of an instance of the class. So that line is defining an instance variable on a different object compared to the rest of the@redirect_urls
uses.It turns out not to matter in your case because the
match
logic is always run before thefailure_message
logic, so@redirect_urls
always has a value when referenced infailure_message
, but if you wanted to keep something like that (e.g. because you find it easier to reason about or whatever), I'd recommend changing from an instance variable to an instance variable -- after all, all the blocks are closures and retain access to local variables.