Stubbing with custom matchers is broken #112

Closed
dchelimsky opened this Issue Jan 24, 2012 · 2 comments

2 participants

@dchelimsky
RSpec member

Originally reported by @aslakhellesoy: rspec/rspec-mocks#97:

I tried to upgrade gherkin's specs from 2.7.0 to 2.8.0, and that broke a bunch of specs. I have isolated it to this:

RSpec::Matchers.define :r do |expected|
  match do |row|
    def row.inspect
      "r " + self.map{|cell| cell}.inspect
    end
    row.map{|cell| cell}.should == expected
  end
end

describe 'Stubbing with custom matcher' do
  it 'works with 2.7.0 but not with 2.8.0' do
    x = Object.new
    x.should_receive(:row).with(r(['1', '']), 1)
    x.should_receive(:row).with(r(['', '4']), 2)

    x.row(['1', ''], 1)
    x.row(['', '4'], 2)
  end
end

I haven't seen anything in the changelog about changing APIs, so I assume this is a regression?

@dchelimsky dchelimsky referenced this issue in rspec/rspec-mocks Jan 24, 2012
Closed

Stubbing with custom matchers is broken #97

@dchelimsky dchelimsky added a commit that closed this issue Jan 24, 2012
@dchelimsky dchelimsky Dup the instance of a DSL generated matcher so its state is not changed
by subsequent invocations.

- Fixes #112.
65d3c4d
@lukeredpath

Just ran into this issue myself. Good to see it is fixed, but I'm wondering that given the intent of the original commit that introduced this bug was to create only one matcher instance and this change effectively reverts the old behaviour by duping the matcher, wouldn't it make more sense to revert that original commit entirely?

IMO the intent was much clearer in the original code, with the explicit creation of a new matcher, rather than using #dup.

In the original bug, you said "Each invocation of the defined method can dup the singleton instance". If it's being duped, then its not really a singleton?

@lukeredpath

Unfortunately, it seems like this change alone has not fixed the problem I'm experiencing. I've opened a new issue, #124.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment