New syntax
Created by: myronmarston
This is my first pass stab at the new syntax discussed in #153 (closed).
It's not ready to merge yet but I wanted to start the code review conversation. Notes:
- I decided to call the old syntax the
:direct
syntax because you call methods directly on the object...and the new syntax the:wrapped
syntax because you wrap the object with a method likeexpect
orallow
. What do others think of these names? - When we introduced the new syntax to rspec-expectations, we decided to default to both syntaxes enabled. This was necessary because we had supported
expect { }.to
for a long time, so it would have been a backwards incompatibility to only enableshould
by default. This time around, I'm thinking of only enabling the existing:direct
syntax by default. Generally, I think projects should pick one syntax or the other, and only use both syntaxes during a transition period. There's no need to enable both syntaxes for backwards compatibility like there was for rspec-expectations. On the other hand, it might encourage more folks to try out the new syntax if both are enabled. I'm kinda on the fence here. - For
any_instance
stubbing, we never figured out what to do for the new syntax. For now, I've gone withexpect_any_instance_of(klass).to receive
andallow_any_instance_of(klass).to receive
but @alindeman put together a gist of some other possibilities to consider. With the current version of this code, the any instance stuff doesn't actually work when you enable only the:wrapped
syntax (it works fine when both syntaxes are enabled). The problem is that the way that any instance works, it recordsshould_receive
andstub
and then plays it back on the instance later...but with the:direct
syntax disabled, the methods aren't there to playback onto. I'm not sure of a fix yet.- I didn't realize this, but the old syntax allowed any object to be
as_null_object
. I personally think that only makes sense on a test double, not on a partial mock. Any objections to only supporting it on test doubles for the new syntax?(On a side note, I need to enable it when only the new syntax is enabled). -
stub_chain
: Do we need to bring this forward to the new syntax? (It's kind a code smell, IMO). - The old syntax supported
obj.stub(:msg_1 => 1, :msg_2 => 2)
but I can't think of a good way to support that with the new syntax. (We'll still supportdouble(:msg_1 => 1, :msg_2 => 2)
, of course). Any objects to not bringing that forward to the new syntax? -
unstub
isn't (yet) supported with the new syntax, and I'm on the fence about whether to support it or not. (There's discussion in #153 (closed) about this). -
When rspec-mocks is used but rspec-expectations is not,(I've implemented this now in ee9d82ee).expect
is not available. It's trivial to write one (it'll be < 10 lines of code both for the method and the class that it instantiates), but I need to figure out how to reliably defineexpect
if and only if it is actually needed, because we don't want it overridingexpect
from rspec-expectations. - Lots of documentation updates are needed.
Please review and give your feedback!