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

Fix any_instance so that it updates already stubbed instances #651

Merged
merged 4 commits into from Apr 6, 2014
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
2 changes: 2 additions & 0 deletions Changelog.md
Expand Up @@ -37,6 +37,8 @@ Bug Fixes:
on an any instance. (Xavier Shay)
* Fix `and_call_original` to work properly when multiple classes in an
inheritance hierarchy have been stubbed with the same method. (Myron Marston)
* Fix `any_instance` so that it updates existing instances that have
already been stubbed. (Myron Marston)

### 3.0.0.beta2 / 2014-02-17
[Full Changelog](http://github.com/rspec/rspec-mocks/compare/v3.0.0.beta1...v3.0.0.beta2)
Expand Down
10 changes: 2 additions & 8 deletions lib/rspec/mocks.rb
Expand Up @@ -67,10 +67,7 @@ def self.teardown
# x = 0
# RSpec::Mocks.allow_message(bar, :foo) { x += 1 }
def self.allow_message(subject, message, opts={}, &block)
orig_caller = opts.fetch(:expected_from) {
CallerFilter.first_non_rspec_line
}
space.proxy_for(subject).add_stub(orig_caller, message, opts, &block)
space.proxy_for(subject).add_stub(message, opts, &block)
end

# Sets a message expectation on `subject`.
Expand All @@ -85,10 +82,7 @@ def self.allow_message(subject, message, opts={}, &block)
# RSpec::Mocks.expect_message(bar, :foo)
# bar.foo
def self.expect_message(subject, message, opts={}, &block)
orig_caller = opts.fetch(:expected_from) {
CallerFilter.first_non_rspec_line
}
space.proxy_for(subject).add_message_expectation(orig_caller, message, opts, &block)
space.proxy_for(subject).add_message_expectation(message, opts, &block)
end

# Call the passed block and verify mocks after it has executed. This allows
Expand Down
1 change: 1 addition & 0 deletions lib/rspec/mocks/any_instance.rb
Expand Up @@ -6,4 +6,5 @@
any_instance/expectation_chain
any_instance/message_chains
any_instance/recorder
any_instance/proxy
].each { |f| RSpec::Support.require_rspec_mocks(f) }
6 changes: 4 additions & 2 deletions lib/rspec/mocks/any_instance/expectation_chain.rb
Expand Up @@ -25,8 +25,10 @@ class PositiveExpectationChain < ExpectationChain

def create_message_expectation_on(instance)
proxy = ::RSpec::Mocks.space.proxy_for(instance)
expected_from = IGNORED_BACKTRACE_LINE
me = proxy.add_message_expectation(expected_from, *@expectation_args, &@expectation_block)
method_name, opts = @expectation_args
Copy link
Member

Choose a reason for hiding this comment

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

👍 I like this change, it really helped clear up what *@expectation_args was doing.

opts = (opts || {}).merge(:expected_form => IGNORED_BACKTRACE_LINE)

me = proxy.add_message_expectation(method_name, opts, &@expectation_block)
if RSpec::Mocks.configuration.yield_receiver_to_any_instance_implementation_blocks?
me.and_yield_receiver_to_implementation
end
Expand Down
112 changes: 112 additions & 0 deletions lib/rspec/mocks/any_instance/proxy.rb
@@ -0,0 +1,112 @@
module RSpec
module Mocks
module AnyInstance
# @private
# The `AnyInstance::Recorder` is responsible for redefining the klass's
# instance method in order to add any stubs/expectations the first time
# the method is called. It's not capable of updating a stub on an instance
# that's already been previously stubbed (either directly, or via
# `any_instance`).
#
# This proxy sits in front of the recorder and delegates both to it
# and to the `RSpec::Mocks::Proxy` for each already mocked or stubbed
# instance of the class, in order to propogates changes to the instances.
#
# Note that unlike `RSpec::Mocks::Proxy`, this proxy class is stateless
# and is not persisted in `RSpec::Mocks.space`.
#
# Proxying for the message expectation fluent interface (typically chained
# off of the return value of one of these methods) is provided by the
# `FluentInterfaceProxy` class below.
Copy link
Member

Choose a reason for hiding this comment

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

damn we have to do some terrible things to support this feature :(

(I don't mean this solution is bad, I just mean :( that we have to do it)

Copy link
Member

Choose a reason for hiding this comment

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

Totally.

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'm totally on the same page :(.

class Proxy
def initialize(recorder, target_proxies)
@recorder = recorder
@target_proxies = target_proxies
end

def klass
@recorder.klass
end

def stub(method_name_or_method_map, &block)
if Hash === method_name_or_method_map
method_name_or_method_map.each do |method_name, return_value|
stub(method_name).and_return(return_value)
end
else
perform_proxying(__method__, [method_name_or_method_map], block) do |proxy|
proxy.add_stub(method_name_or_method_map, &block)
end
end
end

def unstub(method_name)
perform_proxying(__method__, [method_name], nil) do |proxy|
proxy.remove_stub_if_present(method_name)
end
end

def stub_chain(*chain, &block)
perform_proxying(__method__, chain, block) do |proxy|
Mocks::StubChain.stub_chain_on(proxy.object, *chain, &block)
end
end

def expect_chain(*chain, &block)
perform_proxying(__method__, chain, block) do |proxy|
Mocks::ExpectChain.expect_chain_on(proxy.object, *chain, &block)
end
end

def should_receive(method_name, &block)
perform_proxying(__method__, [method_name], block) do |proxy|
proxy.add_message_expectation(method_name, &block)
end
end

def should_not_receive(method_name, &block)
perform_proxying(__method__, [method_name], block) do |proxy|
proxy.add_message_expectation(method_name, &block).never
end
end

private

def perform_proxying(method_name, args, block, &target_proxy_block)
recorder_value = @recorder.__send__(method_name, *args, &block)
proxy_values = @target_proxies.map(&target_proxy_block)
FluentInterfaceProxy.new([recorder_value] + proxy_values)
end
end

# @private
# Delegates messages to each of the given targets in order to
# provide the fluent interface that is available off of message
# expectations when dealing with `any_instance`.
#
# `targets` will typically contain 1 of the `AnyInstance::Recorder`
# return values and N `MessageExpectation` instances (one per instance
# of the `any_instance` klass).
class FluentInterfaceProxy
def initialize(targets)
@targets = targets
end

if RUBY_VERSION.to_f > 1.8
def respond_to_missing?(method_name, include_private = false)
super || @targets.first.respond_to?(method_name, include_private)
end
else
def respond_to?(method_name, include_private = false)
super || @targets.first.respond_to?(method_name, include_private)
end
end

def method_missing(*args, &block)
return_values = @targets.map { |t| t.__send__(*args, &block) }
FluentInterfaceProxy.new(return_values)
end
end
end
end
end
15 changes: 3 additions & 12 deletions lib/rspec/mocks/any_instance/recorder.rb
Expand Up @@ -26,15 +26,9 @@ def initialize(klass)
# instance of this object that invokes the submitted method.
#
# @see Methods#stub
def stub(method_name_or_method_map, &block)
if Hash === method_name_or_method_map
method_name_or_method_map.each do |method_name, return_value|
stub(method_name).and_return(return_value)
end
else
observe!(method_name_or_method_map)
message_chains.add(method_name_or_method_map, StubChain.new(self, method_name_or_method_map, &block))
end
def stub(method_name, &block)
observe!(method_name)
message_chains.add(method_name, StubChain.new(self, method_name, &block))
end

# Initializes the recording a stub chain to be played back against any
Expand Down Expand Up @@ -85,9 +79,6 @@ def unstub(method_name)
raise RSpec::Mocks::MockExpectationError, "The method `#{method_name}` was not stubbed or was already unstubbed"
end
message_chains.remove_stub_chains_for!(method_name)
::RSpec::Mocks.space.proxies_of(@klass).each do |proxy|
stubs[method_name].each { |stub| proxy.remove_single_stub(method_name, stub) }
end
stubs[method_name].clear
stop_observing!(method_name) unless message_chains.has_expectation?(method_name)
end
Expand Down
6 changes: 4 additions & 2 deletions lib/rspec/mocks/any_instance/stub_chain.rb
Expand Up @@ -13,8 +13,10 @@ def expectation_fulfilled?

def create_message_expectation_on(instance)
proxy = ::RSpec::Mocks.space.proxy_for(instance)
expected_from = IGNORED_BACKTRACE_LINE
stub = proxy.add_stub(expected_from, *@expectation_args, &@expectation_block)
method_name, opts = @expectation_args
opts = (opts || {}).merge(:expected_form => IGNORED_BACKTRACE_LINE)

stub = proxy.add_stub(method_name, opts, &@expectation_block)
@recorder.stubs[stub.message] << stub

if RSpec::Mocks.configuration.yield_receiver_to_any_instance_implementation_blocks?
Expand Down
9 changes: 4 additions & 5 deletions lib/rspec/mocks/matchers/receive.rb
Expand Up @@ -9,7 +9,6 @@ def initialize(message, block)
@message = message
@block = block
@recorded_customizations = []
@backtrace_line = CallerFilter.first_non_rspec_line
end

def name
Expand Down Expand Up @@ -62,7 +61,7 @@ def setup_any_instance_allowance(subject, &block)
private

def warn_if_any_instance(expression, subject)
if AnyInstance::Recorder === subject
if AnyInstance::Proxy === subject
RSpec.warning(
"`#{expression}(#{subject.klass}.any_instance).to` " <<
"is probably not what you meant, it does not operate on " <<
Expand All @@ -74,12 +73,12 @@ def warn_if_any_instance(expression, subject)

def setup_mock_proxy_method_substitute(subject, method, block)
proxy = ::RSpec::Mocks.space.proxy_for(subject)
setup_method_substitute(proxy, method, block, @backtrace_line)
setup_method_substitute(proxy, method, block)
end

def setup_any_instance_method_substitute(subject, method, block)
any_instance_recorder = ::RSpec::Mocks.space.any_instance_recorder_for(subject)
setup_method_substitute(any_instance_recorder, method, block)
proxy = ::RSpec::Mocks.space.any_instance_proxy_for(subject)
setup_method_substitute(proxy, method, block)
end

def setup_method_substitute(host, method, block, *args)
Expand Down
8 changes: 4 additions & 4 deletions lib/rspec/mocks/matchers/receive_message_chain.rb
Expand Up @@ -28,14 +28,14 @@ def setup_allowance(subject, &block)
end

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

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

Expand Down
2 changes: 1 addition & 1 deletion lib/rspec/mocks/matchers/receive_messages.rb
Expand Up @@ -58,7 +58,7 @@ def proxy_on(subject)
end

def any_instance_of(subject)
::RSpec::Mocks.space.any_instance_recorder_for(subject)
::RSpec::Mocks.space.any_instance_proxy_for(subject)
end

def each_message_on(host)
Expand Down
7 changes: 3 additions & 4 deletions lib/rspec/mocks/method_double.rb
Expand Up @@ -189,13 +189,12 @@ def add_default_stub(*args, &implementation)
# @private
def remove_stub
raise_method_not_stubbed_error if stubs.empty?
expectations.empty? ? reset : stubs.clear
remove_stub_if_present
end

# @private
def remove_single_stub(stub)
stubs.delete(stub)
restore_original_method if stubs.empty? && expectations.empty?
def remove_stub_if_present
expectations.empty? ? reset : stubs.clear
end

# @private
Expand Down
14 changes: 8 additions & 6 deletions lib/rspec/mocks/proxy.rb
Expand Up @@ -42,7 +42,8 @@ def original_method_handle_for(message)
end

# @private
def add_message_expectation(location, method_name, opts={}, &block)
def add_message_expectation(method_name, opts={}, &block)
location = opts.fetch(:expected_from) { CallerFilter.first_non_rspec_line }
meth_double = method_double_for(method_name)

if null_object? && !block
Expand Down Expand Up @@ -101,7 +102,8 @@ def check_for_unexpected_arguments(expectation)
end

# @private
def add_stub(location, method_name, opts={}, &implementation)
def add_stub(method_name, opts={}, &implementation)
location = opts.fetch(:expected_from) { CallerFilter.first_non_rspec_line }
method_double_for(method_name).add_stub @error_generator, @order_group, location, opts, &implementation
end

Expand All @@ -116,8 +118,8 @@ def remove_stub(method_name)
end

# @private
def remove_single_stub(method_name, stub)
method_double_for(method_name).remove_single_stub(stub)
def remove_stub_if_present(method_name)
method_double_for(method_name).remove_stub_if_present
end

# @private
Expand Down Expand Up @@ -380,7 +382,7 @@ def initialize(order_group)
attr_accessor :warn_about_expectations
alias warn_about_expectations? warn_about_expectations

def add_message_expectation(location, method_name, opts={}, &block)
def add_message_expectation(method_name, opts={}, &block)
warn(method_name) if warn_about_expectations?
super
end
Expand All @@ -390,7 +392,7 @@ def add_negative_message_expectation(location, method_name, &implementation)
super
end

def add_stub(location, method_name, opts={}, &implementation)
def add_stub(method_name, opts={}, &implementation)
warn(method_name) if warn_about_expectations?
super
end
Expand Down
8 changes: 8 additions & 0 deletions lib/rspec/mocks/space.rb
Expand Up @@ -13,6 +13,10 @@ def any_instance_recorder_for(*args)
raise_lifecycle_message
end

def any_instance_proxy_for(*args)
raise_lifecycle_message
end

def register_constant_mutator(mutator)
raise_lifecycle_message
end
Expand Down Expand Up @@ -85,6 +89,10 @@ def any_instance_recorder_for(klass)
end
end

def any_instance_proxy_for(klass)
AnyInstance::Proxy.new(any_instance_recorder_for(klass), proxies_of(klass))
end

def proxies_of(klass)
proxies.values.select { |proxy| klass === proxy.object }
end
Expand Down
7 changes: 2 additions & 5 deletions lib/rspec/mocks/syntax.rb
Expand Up @@ -30,22 +30,19 @@ def self.enable_should(syntax_host = default_should_syntax_host)
syntax_host.class_exec do
def should_receive(message, opts={}, &block)
::RSpec::Mocks::Syntax.warn_unless_should_configured(__method__)
opts[:expected_from] ||= CallerFilter.first_non_rspec_line
::RSpec::Mocks.expect_message(self, message, opts, &block)
end

def should_not_receive(message, &block)
::RSpec::Mocks::Syntax.warn_unless_should_configured(__method__)
opts = {:expected_from => CallerFilter.first_non_rspec_line}
::RSpec::Mocks.expect_message(self, message, opts, &block).never
::RSpec::Mocks.expect_message(self, message, {}, &block).never
Copy link
Member

Choose a reason for hiding this comment

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

Since you default the opts to {} on the message being called, any particular reason you're explicitly setting the empty hash here?

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure I follow...

Copy link
Member

Choose a reason for hiding this comment

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

The third parameter to expect_message:

::RSpec::Mocks.expect_message(self, message, {}, &block).never

vs

::RSpec::Mocks.expect_message(self, message, &block).never

Just a very minor thing, simply a curiosity question.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I now I see what you are getting at. I didn't realize that provided a default for the options hash. It's in a different file so I didn't notice -- I just updated based on what I saw in front of me at the call site.

end

def stub(message_or_hash, opts={}, &block)
::RSpec::Mocks::Syntax.warn_unless_should_configured(__method__)
if ::Hash === message_or_hash
message_or_hash.each {|message, value| stub(message).and_return value }
else
opts[:expected_from] = CallerFilter.first_non_rspec_line
::RSpec::Mocks.allow_message(self, message_or_hash, opts, &block)
end
end
Expand Down Expand Up @@ -80,7 +77,7 @@ def received_message?(message, *args, &block)
Class.class_exec do
def any_instance
::RSpec::Mocks::Syntax.warn_unless_should_configured(__method__)
::RSpec::Mocks.space.any_instance_recorder_for(self)
::RSpec::Mocks.space.any_instance_proxy_for(self)
end
end
end
Expand Down