Skip to content

Don't silently ignore arbitrary method expectations when combining them with 'and_call_original'

gitlab-qa-bot requested to merge warn_when_overriding_implementation into master

Created by: JonRowe

I had a discussion with @myronmarston about a problem I ran into during the conversion of our RSpec suite to the new expect/allow syntax. The gist of it can be found here.

Funnily what I thought was a bug, never quite worked the way I thought it was and I refactored the spec after reading his suggestions. Myron asked me to also add an issue for it, though. So here we go :-)

The spec we talked about had used the following expectation.

CachedUser.should_receive(:where) do |args|
    expect(args[:id]).to have(3).items
    expect(contact_ids).to include(*args[:id])
end.and_call_original

The values passed to 'where' are sort of random so you can't really setup an argument matcher with 'with'. That's why we tried to verify them in the block. The whole code under test there was also part of an AREL call comparable to this one. That's where the 'and_call_original' came into the game

CachedUser.where(id: random_related_user_ids).all

The spec passed. Though, as Myron pointed out, 'and_call_original' completely replaces the block expectation, resulting in the inner expectations never to be executed.

That was a bit surprising to find out, but to be honest also a fault on my side, since I probably never saw the expectation fail in the first place :-/

To make that behavior more obvious I think it would be good to either raise an exception or to output a warning in that case.

Merge request reports