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

Externalize methods and state that was previously held on all objects. #250

Merged
merged 7 commits into from
Mar 28, 2013
42 changes: 11 additions & 31 deletions lib/rspec/mocks/any_instance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,57 +26,37 @@ module AnyInstance
#
# @return [Recorder]
def any_instance
RSpec::Mocks::space.add(self)
modify_dup_to_remove_mock_proxy_when_invoked
AnyInstance.tracked_klasses << self
__recorder
end

# @private
def rspec_verify
__recorder.verify
super
ensure
__recorder.stop_all_observation!
restore_dup
@__recorder = nil
end

# @private
def rspec_reset
restore_dup
__mock_proxy.reset
end

# @private
def __recorder
@__recorder ||= AnyInstance::Recorder.new(self)
end

private
def modify_dup_to_remove_mock_proxy_when_invoked
if method_defined?(:dup) and !method_defined?(:__rspec_original_dup)
class_eval do
def __rspec_dup(*arguments, &block)
clone = __rspec_original_dup(*arguments, &block)
clone.send :__remove_mock_proxy
clone
end

alias_method :__rspec_original_dup, :dup
alias_method :dup, :__rspec_dup
end
def self.verify_all
tracked_klasses.each do |klass|
klass.rspec_verify
end
end

def restore_dup
if method_defined?(:__rspec_original_dup)
class_eval do
alias_method :dup, :__rspec_original_dup
remove_method :__rspec_original_dup
remove_method :__rspec_dup
end
end
def self.reset_all
tracked_klasses.clear
end

def self.tracked_klasses
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just tracked_classes?

Copy link
Member Author

Choose a reason for hiding this comment

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

No particular reason; I'm just in the habit of using klass in the place of class since class is a keyword. Here as part of a longer variable name tracked_classes is fine. I'll update it.

@tracked_klasses ||= []
end
end
end
end

2 changes: 1 addition & 1 deletion lib/rspec/mocks/any_instance/recorder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def stop_all_observation!

# @private
def playback!(instance, method_name)
RSpec::Mocks::space.add(instance)
RSpec::Mocks.space.ensure_registered(instance)
message_chains.playback!(instance, method_name)
@played_methods[method_name] = instance
received_expected_message!(method_name) if message_chains.has_expectation?(method_name)
Expand Down
14 changes: 4 additions & 10 deletions lib/rspec/mocks/extensions/marshal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,12 @@ module Marshal
class << self
def dump_with_mocks(*args)
object = args.shift
return dump_without_mocks(*args.unshift(object)) unless object.instance_variable_defined?(:@mock_proxy)

mp = object.instance_variable_get(:@mock_proxy)
return dump_without_mocks(*args.unshift(object)) unless mp.is_a?(::RSpec::Mocks::Proxy)

object.__send__(:remove_instance_variable, :@mock_proxy)

begin
dump_without_mocks(*args.unshift(object.dup))
ensure
object.instance_variable_set(:@mock_proxy,mp)
unless ::RSpec::Mocks.space.registered?(object)
return dump_without_mocks(*args.unshift(object))
end

dump_without_mocks(*args.unshift(object.dup))
end

alias_method :dump_without_mocks, :dump
Expand Down
23 changes: 0 additions & 23 deletions lib/rspec/mocks/extensions/psych.rb

This file was deleted.

2 changes: 1 addition & 1 deletion lib/rspec/mocks/framework.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
require 'rspec/mocks/errors'
require 'rspec/mocks/error_generator'
require 'rspec/mocks/space'
require 'rspec/mocks/serialization'
require 'rspec/mocks/extensions/marshal'
require 'rspec/mocks/any_instance'
require 'rspec/mocks/mutate_const'
require 'rspec/mocks/stub_chain'
Expand Down
3 changes: 1 addition & 2 deletions lib/rspec/mocks/method_double.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ class << @object; self; end

# @private
def configure_method
RSpec::Mocks::space.add(@object) if RSpec::Mocks::space
warn_if_nil_class
@original_visibility = visibility_for_method
@method_stasher.stash unless @method_is_proxied
Expand All @@ -155,7 +154,7 @@ def define_proxy_method

object_singleton_class.class_eval <<-EOF, __FILE__, __LINE__ + 1
def #{@method_name}(*args, &block)
__mock_proxy.message_received :#{@method_name}, *args, &block
::RSpec::Mocks.space.mock_proxy_for(self).message_received :#{@method_name}, *args, &block
end
#{visibility_for_method}
EOF
Expand Down
44 changes: 9 additions & 35 deletions lib/rspec/mocks/methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ module Methods
# logger.should_receive(:log)
# thing_that_logs.do_something_that_logs_a_message
def should_receive(message, opts={}, &block)
__mock_proxy.add_message_expectation(opts[:expected_from] || caller(1)[0], message.to_sym, opts, &block)
mock_proxy = ::RSpec::Mocks.space.mock_proxy_for(self)
mock_proxy.add_message_expectation(opts[:expected_from] || caller(1)[0], message.to_sym, opts, &block)
end

# Sets and expectation that this object should _not_ receive a message
# during this example.
def should_not_receive(message, &block)
__mock_proxy.add_negative_message_expectation(caller(1)[0], message.to_sym, &block)
mock_proxy = ::RSpec::Mocks.space.mock_proxy_for(self)
mock_proxy.add_negative_message_expectation(caller(1)[0], message.to_sym, &block)
end

# Tells the object to respond to the message with the specified value.
Expand All @@ -32,7 +34,7 @@ def stub(message_or_hash, opts={}, &block)
if Hash === message_or_hash
message_or_hash.each {|message, value| stub(message).and_return value }
else
__mock_proxy.add_stub(caller(1)[0], message_or_hash.to_sym, opts, &block)
::RSpec::Mocks.space.mock_proxy_for(self).add_stub(caller(1)[0], message_or_hash.to_sym, opts, &block)
end
end

Expand All @@ -44,7 +46,7 @@ def stub(message_or_hash, opts={}, &block)
# shared `before` hook for the common case, but you want to replace it
# for a special case.
def unstub(message)
__mock_proxy.remove_stub(message)
::RSpec::Mocks.space.mock_proxy_for(self).remove_stub(message)
end

def stub!(message_or_hash, opts={}, &block)
Expand Down Expand Up @@ -93,7 +95,7 @@ def stub_chain(*chain, &blk)
# returned.
def as_null_object
@_null_object = true
__mock_proxy.as_null_object
::RSpec::Mocks.space.mock_proxy_for(self).as_null_object
end

# Returns true if this object has received `as_null_object`
Expand All @@ -103,37 +105,9 @@ def null_object?

# @private
def received_message?(message, *args, &block)
__mock_proxy.received_message?(message, *args, &block)
end

# @private
def rspec_verify
__mock_proxy.verify
end

# @private
def rspec_reset
__mock_proxy.reset
end

private

def __mock_proxy
@mock_proxy ||= begin
mp = if TestDouble === self
Proxy.new(self, @name, @options)
else
Proxy.new(self)
end

Serialization.fix_for(self)
mp
end
end

def __remove_mock_proxy
@mock_proxy = nil
::RSpec::Mocks.space.mock_proxy_for(self).received_message?(message, *args, &block)
Copy link
Contributor

Choose a reason for hiding this comment

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

I used mock_proxy when it was part of every object in order to avoid a class of potential conflicts. Now that this is nicely decoupled, how about removing the mock_ prefix? Then if we attach proxy_for directly to Mocks we get:

::RSpec::Mocks.proxy_for(self).received_message?(message, *args, &block)

Copy link
Contributor

Choose a reason for hiding this comment

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

By "attach" I mean "redirect to" i.e. it would still be implemented on Space.

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 like how that reads.

end
end
end
end

18 changes: 2 additions & 16 deletions lib/rspec/mocks/mutate_const.rb
Original file line number Diff line number Diff line change
Expand Up @@ -345,27 +345,13 @@ def rspec_reset
def self.mutate(mutator)
register_mutator(mutator)
mutator.mutate
ensure_registered_with_mocks_space
end

# Ensures the constant stubbing is registered with
# rspec-mocks space so that stubbed constants can
# be restored when examples finish.
#
# @api private
def self.ensure_registered_with_mocks_space
return if defined?(@registered_with_mocks_space) && @registered_with_mocks_space
::RSpec::Mocks.space.add(self)
@registered_with_mocks_space = true
end

# Resets all stubbed constants. This is called automatically
# by rspec-mocks when an example finishes.
#
# @api private
def self.rspec_reset
@registered_with_mocks_space = false

def self.reset_all
# We use reverse order so that if the same constant
# was stubbed multiple times, the original value gets
# properly restored.
Expand Down Expand Up @@ -411,4 +397,4 @@ def self.raise_on_invalid_const
# of both stubbing and hiding.
ConstantStubber = ConstantMutator
end
end
end
4 changes: 2 additions & 2 deletions lib/rspec/mocks/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ def warn_about_expectations_on_nil=(new_value)
def allow_message_expectations_on_nil
@warn_about_expectations_on_nil = false

# ensure nil.rspec_verify is called even if an expectation is not set in the example
# ensure nil is verified even if an expectation is not set in the example
# otherwise the allowance would effect subsequent examples
RSpec::Mocks::space.add(nil) unless RSpec::Mocks::space.nil?
RSpec::Mocks.space.ensure_registered(nil) unless RSpec::Mocks.space.nil?
end

# @private
Expand Down
34 changes: 0 additions & 34 deletions lib/rspec/mocks/serialization.rb

This file was deleted.

38 changes: 28 additions & 10 deletions lib/rspec/mocks/space.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,50 @@ module RSpec
module Mocks
# @api private
class Space
def add(obj)
receivers << obj unless receivers.detect {|m| m.equal? obj}
attr_reader :mock_proxies

def initialize
@mock_proxies = {}
end

def verify_all
receivers.each do |mock|
mock.rspec_verify
mock_proxies.values.each do |object|
object.verify
end

AnyInstance.verify_all
end

def reset_all
receivers.each do |mock|
mock.rspec_reset
ConstantMutator.reset_all
AnyInstance.reset_all

mock_proxies.values.each do |object|
object.reset
end
receivers.clear

mock_proxies.clear
expectation_ordering.clear
end

def expectation_ordering
@expectation_ordering ||= OrderGroup.new
end

private
def mock_proxy_for(object)
mock_proxies.fetch(object.object_id) do
mock_proxies[object.object_id] = if TestDouble === object
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this hard to read. I'd either indent the lines below so they are aligned with the if or make it a one-liner:

mock_proxies[object.object_id] = TestDouble === object ? object.__build_mock_proxy : Proxy.new(object)

object.__build_mock_proxy
else
Proxy.new(object)
end
end
end

alias ensure_registered mock_proxy_for

def receivers
@receivers ||= []
def registered?(object)
mock_proxies.has_key?(object.object_id)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rspec/mocks/stub_chain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ def format_chain(*chain, &blk)
end

def find_matching_stub
object.__send__(:__mock_proxy).
__send__(:find_matching_method_stub, chain.first.to_sym)
::RSpec::Mocks.space.mock_proxy_for(object).
__send__(:find_matching_method_stub, chain.first.to_sym)
end
end
end
Expand Down
Loading