Skip to content

Commit

Permalink
Fix caching bugs in DSL matchers.
Browse files Browse the repository at this point in the history
Previously, `@messages` was not cleared when new instances
were created, so the `description`, `failure_message_for_should`
and `failure_message_for_should_not` blocks would take on the
value set by the most recently built instance.

In a case like:

RSpec::Matchers.define(:be_like_a) do |expected|
  description { "be like a #{expected}" }
end

be_like_a_moose = be_like_a("moose")
be_like_a_horse = be_like_a("horse")

...each matcher instance eval'd the `define` block once.
Since the object that holds a reference to the description
block (the `@messages` instance variable) was not cleared
between the two examples, the description block for both
instances would have `expected` bound to "horse" (since
it was instantiated last, and its define block eval'd last).

This caused the description and failure messages to return
the wrong values in situations like these.
  • Loading branch information
myronmarston committed Feb 7, 2013
1 parent 569219c commit fc4b66d
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 1 deletion.
3 changes: 3 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ Bug fixes
actually verify anything (Myron Marston).
* Add compatibility for diff-lcs 1.2 and relax the version
constraint (Peter Goldstein).
* Fix DSL-generated matchers to allow multiple instances of the
same matcher in the same example to have different description
and failure messages based on the expected value (Myron Marston).

Enhancements

Expand Down
3 changes: 2 additions & 1 deletion lib/rspec/matchers/matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def initialize(name, &declarations)
end

PERSISTENT_INSTANCE_VARIABLES = [
:@name, :@declarations, :@diffable, :@messages,
:@name, :@declarations, :@diffable,
:@match_block, :@match_for_should_not_block,
:@expected_exception
].to_set
Expand All @@ -37,6 +37,7 @@ def for_expected(*expected)
instance_variables.map {|ivar| ivar.intern}.each do |ivar|
instance_variable_set(ivar, nil) unless (PERSISTENT_INSTANCE_VARIABLES + [:@expected]).include?(ivar)
end
@messages = {}
making_declared_methods_public do
instance_eval_with_args(*@expected, &@declarations)
end
Expand Down
38 changes: 38 additions & 0 deletions spec/rspec/matchers/matcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,44 @@ def second_word
expect(matcher.matches?(8)).to be_true
end

context 'when multiple instances of the same matcher are used in the same example' do
RSpec::Matchers.define(:be_like_a) do |expected|
match { |actual| actual == expected }
description { "be like a #{expected}" }
failure_message_for_should { "expected to be like a #{expected}" }
failure_message_for_should_not { "expected not to be like a #{expected}" }
end

# Note: these bugs were only exposed when creating both instances
# first, then checking their descriptions/failure messages.
#
# That's why we eager-instantiate them here.
let!(:moose) { be_like_a("moose") }
let!(:horse) { be_like_a("horse") }

it 'allows them to use the expected value in the description' do
expect(horse.description).to eq("be like a horse")
expect(moose.description).to eq("be like a moose")
end

it 'allows them to use the expected value in the positive failure message' do
expect(moose.failure_message_for_should).to eq("expected to be like a moose")
expect(horse.failure_message_for_should).to eq("expected to be like a horse")
end

it 'allows them to use the expected value in the negative failure message' do
expect(moose.failure_message_for_should_not).to eq("expected not to be like a moose")
expect(horse.failure_message_for_should_not).to eq("expected not to be like a horse")
end

it 'allows them to match separately' do
expect("moose").to moose
expect("horse").to horse
expect("horse").not_to moose
expect("moose").not_to horse
end
end

describe "#match_unless_raises" do
context "with an assertion" do
let(:mod) do
Expand Down

0 comments on commit fc4b66d

Please sign in to comment.