Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Clear out user-defined instance variables between invocations of DSL-…

…defined matchers.

- Fixes #104.
  • Loading branch information...
commit f4efada6c86b840802a18e3de12ea68279f9030f 1 parent 95bf234
@dchelimsky dchelimsky authored
View
4 Changelog.md
@@ -8,7 +8,9 @@ Enhancements
Bug fixes
-* Align respond_to? and method_missing in matchers generated by DSL.
+* Align respond_to? and method_missing in DSL-defined matchers.
+* Clear out user-defined instance variables between invocations of DSL-defined
+ matchers.
### 2.8.0 / 2012-01-04
View
9 lib/rspec/matchers/matcher.rb
@@ -26,8 +26,17 @@ def initialize(name, &declarations)
}
end
+ PERSISENT_INSTANCE_VARIABLES = [
+ :@name, :@declarations, :@diffable, :@messages,
+ :@match_block, :@match_for_should_not_block,
+ :@expected_exception
+ ]
+
# @api private
def for_expected(*expected)
+ instance_variables.each do |ivar|
+ instance_variable_set(ivar, nil) unless PERSISENT_INSTANCE_VARIABLES.include?(ivar)
@myronmarston Owner

One suggestion: Array#include? is O(N), whereas Set#include? is O(1). If I need a collection to check membership, I tend to favor Set over Array.

That said, it's only 7 items in the collection, so I wouldn't expect it to make a big difference.

@dchelimsky Owner

I'm done for the night, but if you want to change it to a set please feel free.

@myronmarston Owner

Done:

40f83a9

@wfarr
wfarr added a note

Is there any particular reason to set the ivar to nil rather than unsetting it via remove_instance_variable?

This could still potentially be a breaking change for anything using tis DSL that still might make use of instance_variables.include? or defined?.

@dchelimsky Owner

It was a convenience in order to not differentiate between user ivars and internal ivars that need to be reset (but present) between invocations. I'll change it so it resets the internally resettable ivars but removes the user ivars.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
@expected = expected
making_declared_methods_public do
instance_eval_with_args(*@expected, &@declarations)
View
12 spec/rspec/matchers/dsl_spec.rb
@@ -15,6 +15,18 @@ def question?
expect { ignore.i_dont_exist }.to raise_error(NameError)
end
+ it "clears user instance variables between invocations" do
+ RSpec::Matchers::define(:be_just_like) do |expected|
+ match do |actual|
+ @foo ||= expected
+ @foo == actual
+ end
+ end
+
+ 3.should be_just_like(3)
+ 4.should be_just_like(4)
+ end
+
describe "#respond_to?" do
it "returns true for methods in example scope" do
RSpec::Matchers::define(:ignore) {}
@myronmarston

One suggestion: Array#include? is O(N), whereas Set#include? is O(1). If I need a collection to check membership, I tend to favor Set over Array.

That said, it's only 7 items in the collection, so I wouldn't expect it to make a big difference.

@dchelimsky

I'm done for the night, but if you want to change it to a set please feel free.

@wfarr

Is there any particular reason to set the ivar to nil rather than unsetting it via remove_instance_variable?

This could still potentially be a breaking change for anything using tis DSL that still might make use of instance_variables.include? or defined?.

@dchelimsky

It was a convenience in order to not differentiate between user ivars and internal ivars that need to be reset (but present) between invocations. I'll change it so it resets the internally resettable ivars but removes the user ivars.

Please sign in to comment.
Something went wrong with that request. Please try again.