[rspec-mocks] Proposed change: Make *and_call_original* the default behavior for *receive*

[Brian John]

I've been using rspec for a few years now and one thing has bothered me since the switch to the new expect syntax. For partial mocks, when using allow/expect(something).to receive... it reads more like a spy to me than a stub. Using an example from the README:

expect(Person).to receive(:find)

Reading this like a sentence, I would expect Person to receive a call to find...and that's it. Nothing in the above indicates to me that the find method should also be stubbed. The stub behavior seems like a side effect. 

My proposal would be to change the default behavior of receive so that and_call_original is the default behavior. An additional method could be added to to trigger the stub behavior. So the equivalent behavior for the above example would read something like this:

expect(Person).to receive(:find).and_stub

Obviously making this change would be relatively painful in that:

* some may not agree that this is the correct behavior

* those that are used to the prior behavior would have to get used to the new behavior

* it would break a lot of existing tests

Given the above I wanted to post here to see how others felt before submitting a pull request. My apologies if this has already been discussed, I did a search of the issues in the rspec-mocks repository and this forum and I couldn't find anything.

[Jon Rowe]

Personally, given than spec-mocks is primarily intended for creating test doubles and stubbing objects I feel this would be a perverse default, in the limited cases where stubbing a real object is a good idea you almost never want this to happen, I can kind of see this sort of behaviour might be expected in rspec-expectations but I don’t feel like it should be the default in mocks.

Remember that the majority of `expect(something).to receive` calls are going to be in cases where something is a double and has no real behaviour in the first place.

Jon Rowe

---------------------------

ma...@jonrowe.co.uk

jonrowe.co.uk

--
You received this message because you are subscribed to the Google Groups "rspec" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rspec+un...@googlegroups.com.
To post to this group, send email to rs...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rspec/8d5d5525-5749-4284-b8d7-e38833758d13%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Myron Marston]

Thanks for the suggestion! I agree with Jon, though, for a couple reasons:

• Consistency: currently the default behavior is the same for expect and allow against all types of test doubles (including partial doubles). Changing the default behavior for one specific situations (using expect on a partial double) is, IMO, quite confusing. Consider, for example, this bit of code: expect(foo).to receive(:bar); foo.bar. If we made the change, what would it do? It would depend on what foo was, which might not be obvious from looking at the example in isolation (foo might be provided by a helper method defined in a module in a separate file or something).
• Safer: in many cases, people are mocking or stubbing a method to prevent it from doing something they don’t want to happen in a test. If the default was and_call_original, it would surprisingly do the thing they are trying to prevent, which could have dangerous consequences. This would especially be a problem with long time RSpec users who expect the existing behavior and aren’t aware of the change.
• Inertia: this would be a massive breaking change for most folks and for every person who wants this change (like yourself) I expect we’d have at least 2 who would be against it.

Myron

​

To view this discussion on the web visit https://groups.google.com/d/msgid/rspec/E66977996C89404F837AD40B422F324A%40jonrowe.co.uk.