Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allow(...).to receive_message_chain #467

Merged
merged 1 commit into from Nov 18, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions Changelog.md
Expand Up @@ -10,6 +10,12 @@ Bug Fixes:
* Fix regression in 3.0.0.beta1 that caused `double("string_name" => :value)`
to stop working. (Xavier Shay)

Enhancements:

* Add receive_message_chain which provides the functionality of the old
stub_chain for the new allow/expect syntax. Use it like so: allow(...).to
receive_message_chain(:foo, :bar, :bazz). (Sam Phippen).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little odd that the primary usage of this (e.g. allow(...).to receive_message_chain(...)) isn't highlighted here, and instead the expect form is. Also, calling receive_message_chain a "matcher" is stretching it (even though it implements the rspec-expectations matcher protocol). I'd probably phrase this something like:

  • Add receive_message_chain which provides the functionality of the old stub_chain for the new allow/expect syntax. Use it like so: allow(...).to receive_message_chain(:foo, :bar, :bazz). (Sam Phippen).

### 3.0.0.beta1 / 2013-11-07
[full changelog](http://github.com/rspec/rspec-mocks/compare/v2.99.0.beta1...v3.0.0.beta1)

Expand Down
49 changes: 49 additions & 0 deletions features/message_expectations/message_chains_using_expect.feature
@@ -0,0 +1,49 @@
Feature: Message chains in the expect syntax

You can use `receive_message_chain` to stub nested calls
on both partial and pure mock objects.

Scenario: allow a chained message
Given a file named "spec/chained_messages.rb" with:
"""ruby
describe "a chained message expectation" do
it "passes if the expected number of calls happen" do
d = double
allow(d).to receive_message_chain(:to_a, :length)

d.to_a.length
end
end
"""
When I run `rspec spec/chained_messages.rb`
Then the output should contain "1 example, 0 failures"

Scenario: allow a chained message with a return value
Given a file named "spec/chained_messages.rb" with:
"""ruby
describe "a chained message expectation" do
it "passes if the expected number of calls happen" do
d = double
allow(d).to receive_message_chain(:to_a, :length).and_return(3)

expect(d.to_a.length).to eq(3)
end
end
"""
When I run `rspec spec/chained_messages.rb`
Then the output should contain "1 example, 0 failures"

Scenario: expect a chained message with a return value
Given a file named "spec/chained_messages.rb" with:
"""ruby
describe "a chained message expectation" do
it "passes if the expected number of calls happen" do
d = double
expect(d).to receive_message_chain(:to_a, :length).and_return(3)

expect(d.to_a.length).to eq(3)
end
end
"""
When I run `rspec spec/chained_messages.rb`
Then the output should contain "1 example, 0 failures"
35 changes: 35 additions & 0 deletions lib/rspec/mocks/any_instance/expect_chain_chain.rb
@@ -0,0 +1,35 @@
module RSpec
module Mocks
module AnyInstance
# @private
class ExpectChainChain < StubChain
def initialize(*args)
super
@expectation_fulfilled = false
end

def expectation_fulfilled?
@expectation_fulfilled
end

def playback!(instance)
super.tap { @expectation_fulfilled = true }
end

private

def create_message_expectation_on(instance)
::RSpec::Mocks::ExpectChain.expect_chain_on(instance, *@expectation_args, &@expectation_block)
end

def invocation_order
@invocation_order ||= {
:and_return => [nil],
:and_raise => [nil],
:and_yield => [nil]
}
end
end
end
end
end
9 changes: 9 additions & 0 deletions lib/rspec/mocks/any_instance/recorder.rb
Expand Up @@ -49,6 +49,15 @@ def stub_chain(*method_names_and_optional_return_values, &block)
end
end

# @api private
def expect_chain(*method_names_and_optional_return_values, &block)
@expectation_set = true
normalize_chain(*method_names_and_optional_return_values) do |method_name, args|
observe!(method_name)
message_chains.add(method_name, ExpectChainChain.new(self, *args, &block))
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to add this. IMO, one win of the new syntax is that things compose nicely so that expecting a chain falls out of it naturally. I want to encourage folks to upgrade, so I don't see a reason to backport the feature to the old syntax.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind: I see now that this is needed for your implementation. I originally thought that you had added it so that folks could do SomeClass.any_instance.expect_chain, which I'm not very interested in supporting.

This needs some kind of YARD docs. I'd prefer to have it labeled @api private so that it's officially labeled as a non-public API.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now labelled as api private.


# Initializes the recording a message expectation to be played back
# against any instance of this object that invokes the submitted
# method.
Expand Down
4 changes: 4 additions & 0 deletions lib/rspec/mocks/any_instance/stub_chain_chain.rb
Expand Up @@ -3,6 +3,10 @@ module Mocks
module AnyInstance
# @private
class StubChainChain < StubChain
def initialize(*args)
super
@expectation_fulfilled = false
end

private

Expand Down
6 changes: 3 additions & 3 deletions lib/rspec/mocks/error_generator.rb
Expand Up @@ -117,9 +117,9 @@ def raise_wrong_arity_error(args_to_yield, arity)
end

# @private
def raise_only_valid_on_a_partial_mock(method)
__raise "#{intro} is a pure mock object. `#{method}` is only " +
"available on a partial mock object."
def raise_only_valid_on_a_partial_double(method)
__raise "#{intro} is a pure test double. `#{method}` is only " +
"available on a partial double."
end

# @private
Expand Down
4 changes: 3 additions & 1 deletion lib/rspec/mocks/framework.rb
Expand Up @@ -21,14 +21,16 @@
require 'rspec/mocks/any_instance/chain'
require 'rspec/mocks/any_instance/stub_chain'
require 'rspec/mocks/any_instance/stub_chain_chain'
require 'rspec/mocks/any_instance/expect_chain_chain'
require 'rspec/mocks/any_instance/expectation_chain'
require 'rspec/mocks/any_instance/message_chains'
require 'rspec/mocks/any_instance/recorder'
require 'rspec/mocks/mutate_const'
require 'rspec/mocks/matchers/have_received'
require 'rspec/mocks/matchers/receive'
require 'rspec/mocks/matchers/receive_messages'
require 'rspec/mocks/stub_chain'
require 'rspec/mocks/matchers/receive_message_chain'
require 'rspec/mocks/message_chain'
require 'rspec/mocks/targets'
require 'rspec/mocks/syntax'
require 'rspec/mocks/configuration'
Expand Down
24 changes: 12 additions & 12 deletions lib/rspec/mocks/matchers/receive.rb
Expand Up @@ -27,7 +27,7 @@ def setup_expectation(subject, &block)
def setup_negative_expectation(subject, &block)
# ensure `never` goes first for cases like `never.and_return(5)`,
# where `and_return` is meant to raise an error
@recorded_customizations.unshift Customization.new(:never, [], nil)
@recorded_customizations.unshift ExpectationCustomization.new(:never, [], nil)

warn_if_any_instance("expect", subject)

Expand Down Expand Up @@ -56,7 +56,7 @@ def setup_any_instance_allowance(subject, &block)
next if method_defined?(method)

define_method(method) do |*args, &block|
@recorded_customizations << Customization.new(method, args, block)
@recorded_customizations << ExpectationCustomization.new(method, args, block)
self
end
end
Expand Down Expand Up @@ -93,18 +93,18 @@ def setup_method_substitute(host, method, block, *args)
end
expectation
end
end
end

class Customization
def initialize(method_name, args, block)
@method_name = method_name
@args = args
@block = block
end
class ExpectationCustomization
def initialize(method_name, args, block)
@method_name = method_name
@args = args
@block = block
end

def playback_onto(expectation)
expectation.__send__(@method_name, *@args, &@block)
end
end
def playback_onto(expectation)
expectation.__send__(@method_name, *@args, &@block)
end
end
end
Expand Down
65 changes: 65 additions & 0 deletions lib/rspec/mocks/matchers/receive_message_chain.rb
@@ -0,0 +1,65 @@
module RSpec
module Mocks
module Matchers
#@api private
class ReceiveMessageChain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A big part of being SemVer compliant is being explicit about what is part of the public API and what is not. Every class should have yard comments documenting that. I would make this @api private since it's not intended to be instantiated directly by end users.

def initialize(chain, &block)
@chain = chain
@block = block
@recorded_customizations = []
end

[:and_return, :and_throw, :and_raise, :and_yield, :and_call_original].each do |msg|
define_method(msg) do |*args, &block|
@recorded_customizations << ExpectationCustomization.new(msg, args, block)
self
end
end

def name
"receive_message_chain"
end

def setup_allowance(subject, &block)
chain = StubChain.stub_chain_on(subject, *@chain, &(@block || block))
replay_customizations(chain)
end

def setup_any_instance_allowance(subject, &block)
recorder = ::RSpec::Mocks.any_instance_recorder_for(subject)
chain = recorder.stub_chain(*@chain, &(@block || block))
replay_customizations(chain)
end

def setup_any_instance_expectation(subject, &block)
recorder = ::RSpec::Mocks.any_instance_recorder_for(subject)
chain = recorder.expect_chain(*@chain, &(@block || block))
replay_customizations(chain)
end

def setup_expectation(subject, &block)
chain = ExpectChain.expect_chain_on(subject, *@chain, &(@block || block))
replay_customizations(chain)
end

def setup_negative_expectation(*args)
raise NegationUnsupportedError.new(
"`expect(...).not_to receive_message_chain` is not supported " +
"since it doesn't really make sense. What would it even mean?"
)
end

alias matches? setup_expectation
alias does_not_match? setup_negative_expectation

private

def replay_customizations(chain)
@recorded_customizations.each do |customization|
customization.playback_onto(chain)
end
end
end
end
end
end
91 changes: 91 additions & 0 deletions lib/rspec/mocks/message_chain.rb
@@ -0,0 +1,91 @@
module RSpec
module Mocks
# @private
class MessageChain
attr_reader :object, :chain, :block

def initialize(object, *chain, &blk)
@object = object
@chain, @block = format_chain(*chain, &blk)
end

# @api private
def setup_chain
if chain.length > 1
if matching_stub = find_matching_stub
chain.shift
chain_on(matching_stub.invoke(nil), *chain, &@block)
elsif matching_expectation = find_matching_expectation
chain.shift
chain_on(matching_expectation.invoke_without_incrementing_received_count(nil), *chain, &@block)
else
next_in_chain = Double.new
expectation(object, chain.shift, next_in_chain)
chain_on(next_in_chain, *chain, &@block)
end
else
::RSpec::Mocks.allow_message(object, chain.shift, {}, &block)
end
end

private

def expectation(object, message, returned_object)
raise NotImplementedError.new
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally like this pattern when end users are meant to subclass (as the definition with NotImplementedError makes it clear that they are to define it in their subclass), but in this case I'm in favor of not having this, for a few reasons:

  • This is only meant to be subclassed by us, and we've implemented the override in both cases.
  • This code will never be executed.
  • Every method def adds to the amount of memory the ruby process uses (and the amount of time it takes to boot) -- so while this one method def probably won't be noticable, I prefer not to add method defs that will never be called, as they can add up over time.

What do you think about removing this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


def chain_on(object, *chain, &block)
initialize(object, *chain, &block)
setup_chain
end

def format_chain(*chain, &blk)
if Hash === chain.last
hash = chain.pop
hash.each do |k,v|
chain << k
blk = lambda { v }
end
end
return chain.join('.').split('.'), blk
end

def find_matching_stub
::RSpec::Mocks.proxy_for(object).
__send__(:find_matching_method_stub, chain.first.to_sym)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't notice this before...but it looks like this needs to be defined in the subclasses as well: there's find_matching_method_stub and find_matching_expectation. Before fixing this, it'd be good to add a failing test first. Reading the code, I think it would come into play in a case like this:

expect(obj).to receive(:foo)
expect(obj).to receive_message_chain(:foo, :bar, :bazz)

I'm not 100% sure on that, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a look.

end

def find_matching_expectation
::RSpec::Mocks.proxy_for(object).
__send__(:find_matching_expectation, chain.first.to_sym)
end
end

# @private
class ExpectChain < MessageChain
# @api private
def self.expect_chain_on(object, *chain, &blk)
new(object, *chain, &blk).setup_chain
end

private

def expectation(object, message, returned_object)
::RSpec::Mocks.expect_message(object, message, {}) { returned_object }
end
end

# @private
class StubChain < MessageChain
def self.stub_chain_on(object, *chain, &blk)
new(object, *chain, &blk).setup_chain
end

private

def expectation(object, message, returned_object)
::RSpec::Mocks.allow_message(object, message, {}) { returned_object }
end
end
end
end