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

Reduce duplication in & between Mock & ObjectMethods #431

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
22c594f
make arg iteration similar in expects & stubs
nitishr Nov 29, 2019
ffac2ad
Refactor: extract add_expectation to DRY expects & stubs
nitishr Nov 29, 2019
f700c36
Refactor: inline temp - expectation
nitishr Nov 29, 2019
da280b1
Refactor: inline temp - iterator
nitishr Nov 29, 2019
7579416
Refactor: extract anticipates to DRY expects,stubs
nitishr Nov 29, 2019
7cff8fa
Refactor: inline temp - method
nitishr Nov 29, 2019
7494b2d
Refactor: extract stub_method to be moved to Mockery
nitishr Nov 29, 2019
5f83af5
Refactor: extract & reuse stubbed_method
nitishr Nov 29, 2019
b9422ba
Refactor: move stub_method to Mockery from ObjectMethods
nitishr Nov 29, 2019
7a691d9
Refactor: make on_stubbing{,_method_unnecessarily} private
nitishr Nov 29, 2019
bf32420
Refactor: rename on_stubbingx to check_stubbingx
nitishr Nov 29, 2019
73cf959
Refactor: no need for stateful obj for arg iter-n
nitishr Nov 29, 2019
b34726e
substitute algorithm for ArgumentIterator.each
nitishr Nov 30, 2019
547a4d5
Refactor: extract anticipates to DRY expects,stubs
nitishr Nov 30, 2019
bf7ef4e
Refactor: inline add_expectation
nitishr Nov 30, 2019
84e3ee7
no need to pass caller around
nitishr Nov 30, 2019
bfd20d8
ArgumentIterator.each returns the last result anyway
nitishr Nov 30, 2019
9761be3
Refactor: inline temp - mocker to restrict scope
nitishr Nov 30, 2019
b2354df
call Mock#{expects,stubs} with methods_vs_return_values
nitishr Dec 1, 2019
7e7a61b
Refactor: prep to move by inlining temp - method
nitishr Dec 1, 2019
b87733f
Refactor: replace ending yield w/ call in sequence
nitishr Dec 1, 2019
2c4a5db
call stub_method alongside expectation setting for each method
nitishr Dec 1, 2019
262ea4c
add expectation to expectations after fully set up
nitishr Dec 1, 2019
5a5ed50
Refactor: rename stubbed_method->stubba_method_for
nitishr Dec 1, 2019
5f1c5f4
Refactor: move ArgumentIterator#each to Mock
nitishr Dec 2, 2019
365884e
Refactor: inline each_argument
nitishr Dec 2, 2019
19004e7
Refactor: DRY up {PRE_RUBY_V19,RUBY_V19_AND_LATER}_EXCLUDED_METHODS
nitishr Dec 2, 2019
201cfaa
Refactor: extract anticipates to DRY expects,stubs
nitishr Dec 2, 2019
2840411
Refactor: inline method error_if_frozen
nitishr Dec 2, 2019
583103b
more consistent setting of multiple expectations
nitishr Feb 19, 2020
b80cb98
move Mockery#stub_method call to ObjectMethods
nitishr Feb 20, 2020
8a7af66
remove the Spanish Inquisition special case
nitishr Feb 21, 2020
9930e5a
Refactor: inline anticipates into expects
nitishr Feb 21, 2020
1906a10
Refactor: ExpectationSetting->CompositeExpectation
nitishr Feb 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions lib/mocha/argument_iterator.rb

This file was deleted.

18 changes: 18 additions & 0 deletions lib/mocha/composite_expectation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module Mocha
class CompositeExpectation
attr_reader :expectations

def initialize(expectations)
@expectations = expectations
end

%w[
times twice once never at_least at_least_once at_most at_most_once
with yields multiple_yields returns raises throws then when in_sequence
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remember to add with_block_given and with_no_block_given when merging

].each do |method_name|
define_method(method_name) do |*args, &block|
CompositeExpectation.new(@expectations.map { |e| e.send(method_name, *args, &block) })
end
end
end
end
19 changes: 6 additions & 13 deletions lib/mocha/mock.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
require 'mocha/singleton_class'
require 'mocha/expectation'
require 'mocha/expectation_list'
require 'mocha/composite_expectation'
require 'mocha/invocation'
require 'mocha/names'
require 'mocha/receivers'
require 'mocha/method_matcher'
require 'mocha/parameters_matcher'
require 'mocha/argument_iterator'
require 'mocha/expectation_error_factory'
require 'mocha/ruby_version'

Expand Down Expand Up @@ -109,14 +109,15 @@ class Mock
# object.expects(:expected_method_one).returns(:result_one)
# object.expects(:expected_method_two).returns(:result_two)
def expects(method_name_or_hash, backtrace = nil)
iterator = ArgumentIterator.new(method_name_or_hash)
iterator.each do |*args|
CompositeExpectation.new(Array(method_name_or_hash).map do |*args|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the documentation:

@return [Expectation] last-built expectation which can be further modified by methods on {Expectation}

This introduces a breaking change. So, even if we agree that the new behavior is preferable, we'd have to think about deprecating the old behavior.

There are no existing tests to check for the documented behavior, though.

args = args.flatten
method_name = args.shift
yield method_name if block_given?
ensure_method_not_already_defined(method_name)
expectation = Expectation.new(self, method_name, backtrace)
expectation.returns(args.shift) unless args.empty?
@expectations.add(expectation)
end
end)
end

# Adds an expectation that the specified method may be called any number of times with any parameters.
Expand Down Expand Up @@ -145,15 +146,7 @@ def expects(method_name_or_hash, backtrace = nil)
# object.stubs(:stubbed_method_one).returns(:result_one)
# object.stubs(:stubbed_method_two).returns(:result_two)
def stubs(method_name_or_hash, backtrace = nil)
iterator = ArgumentIterator.new(method_name_or_hash)
iterator.each do |*args|
method_name = args.shift
ensure_method_not_already_defined(method_name)
expectation = Expectation.new(self, method_name, backtrace)
expectation.at_least(0)
expectation.returns(args.shift) unless args.empty?
@expectations.add(expectation)
end
expects(method_name_or_hash, backtrace).at_least(0)
end

# Removes the specified stubbed methods (added by calls to {#expects} or {#stubs}) and all expectations associated with them.
Expand Down
27 changes: 16 additions & 11 deletions lib/mocha/mockery.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ def new_state_machine(name)
add_state_machine(StateMachine.new(name))
end

def stub_method(object, method_name)
check_stubbing(object, method_name)
stubba.stub(object.stubba_method_for(method_name))
end

def verify(assertion_counter = nil)
unless mocks.all? { |mock| mock.__verified__?(assertion_counter) }
message = "not all expectations were satisfied\n#{mocha_inspect}"
Expand All @@ -95,7 +100,7 @@ def verify(assertion_counter = nil)
expectations.each do |e|
unless Mocha.configuration.stubbing_method_unnecessarily == :allow
next if e.used?
on_stubbing_method_unnecessarily(e)
check_stubbing_method_unnecessarily(e)
end
end
end
Expand Down Expand Up @@ -125,7 +130,15 @@ def mocha_inspect
message
end

def on_stubbing(object, method)
attr_writer :logger

def logger
@logger ||= Logger.new($stderr)
end

private

def check_stubbing(object, method)
method = PRE_RUBY_V19 ? method.to_s : method.to_sym
method_signature = "#{object.mocha_inspect}.#{method}"
check(:stubbing_non_existent_method, 'non-existent method', method_signature) do
Expand All @@ -138,18 +151,10 @@ def on_stubbing(object, method)
check(:stubbing_method_on_non_mock_object, 'method on non-mock object', method_signature)
end

def on_stubbing_method_unnecessarily(expectation)
def check_stubbing_method_unnecessarily(expectation)
check(:stubbing_method_unnecessarily, 'method unnecessarily', expectation.method_signature, expectation.backtrace)
end

attr_writer :logger

def logger
@logger ||= Logger.new($stderr)
end

private

def check(action, description, method_signature, backtrace = caller)
return if block_given? && !yield
message = "stubbing #{description}: #{method_signature}"
Expand Down
41 changes: 9 additions & 32 deletions lib/mocha/object_methods.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
require 'mocha/mockery'
require 'mocha/instance_method'
require 'mocha/argument_iterator'
require 'mocha/expectation_error_factory'

module Mocha
Expand Down Expand Up @@ -40,6 +39,11 @@ def stubba_class
singleton_class
end

# @private
def stubba_method_for(method_name)
stubba_method.new(stubba_object, method_name)
end

# Adds an expectation that the specified method must be called exactly once with any parameters.
#
# The original implementation of the method is replaced during the test and then restored at the end of the test. The temporary replacement method has the same visibility as the original method.
Expand Down Expand Up @@ -69,24 +73,12 @@ def stubba_class
#
# @see Mock#expects
def expects(expected_methods_vs_return_values)
if expected_methods_vs_return_values.to_s =~ /the[^a-z]*spanish[^a-z]*inquisition/i
raise ExpectationErrorFactory.build('NOBODY EXPECTS THE SPANISH INQUISITION!')
end
if frozen?
raise StubbingError.new("can't stub method on frozen object: #{mocha_inspect}", caller)
end
expectation = nil
mockery = Mocha::Mockery.instance
iterator = ArgumentIterator.new(expected_methods_vs_return_values)
iterator.each do |*args|
method_name = args.shift
mockery.on_stubbing(self, method_name)
method = stubba_method.new(stubba_object, method_name)
mockery.stubba.stub(method)
expectation = mocha.expects(method_name, caller)
expectation.returns(args.shift) unless args.empty?
mocha.expects(expected_methods_vs_return_values, caller) do |method_name|
Mockery.instance.stub_method(self, method_name)
end
expectation
end

# Adds an expectation that the specified method may be called any number of times with any parameters.
Expand Down Expand Up @@ -118,21 +110,7 @@ def expects(expected_methods_vs_return_values)
#
# @see Mock#stubs
def stubs(stubbed_methods_vs_return_values)
if frozen?
raise StubbingError.new("can't stub method on frozen object: #{mocha_inspect}", caller)
end
expectation = nil
mockery = Mocha::Mockery.instance
iterator = ArgumentIterator.new(stubbed_methods_vs_return_values)
iterator.each do |*args|
method_name = args.shift
mockery.on_stubbing(self, method_name)
method = stubba_method.new(stubba_object, method_name)
mockery.stubba.stub(method)
expectation = mocha.stubs(method_name, caller)
expectation.returns(args.shift) unless args.empty?
end
expectation
expects(stubbed_methods_vs_return_values).at_least(0)
end

# Removes the specified stubbed methods (added by calls to {#expects} or {#stubs}) and all expectations associated with them.
Expand Down Expand Up @@ -161,8 +139,7 @@ def stubs(stubbed_methods_vs_return_values)
def unstub(*method_names)
mockery = Mocha::Mockery.instance
method_names.each do |method_name|
method = stubba_method.new(stubba_object, method_name)
mockery.stubba.unstub(method)
mockery.stubba.unstub(stubba_method_for(method_name))
end
end
end
Expand Down
25 changes: 25 additions & 0 deletions test/acceptance/expectations_on_multiple_methods_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,29 @@ def my_instance_method_2
end
assert_passed(test_result)
end

def test_should_configure_expectations_for_multiple_methods
instance = Class.new do
def my_instance_method_1
:original_return_value_1
end

def my_instance_method_2
:original_return_value_2
end
end.new
test_result = run_as_test do
instance.stubs(
:my_instance_method_1 => :new_return_value_1,
:my_instance_method_2 => :new_return_value_2
).at_least(2)
assert_equal :new_return_value_1, instance.my_instance_method_1
assert_equal :new_return_value_2, instance.my_instance_method_2
end
assert_failed(test_result)
assert_equal [
"- expected at least twice, invoked once: #{instance.mocha_inspect}.my_instance_method_1(any_parameters)",
"- expected at least twice, invoked once: #{instance.mocha_inspect}.my_instance_method_2(any_parameters)"
], test_result.failure_message_lines[-2..-1].sort
end
end
38 changes: 12 additions & 26 deletions test/unit/mock_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def test_should_build_and_store_expectations
mock = build_mock
expectation = mock.expects(:method1)
assert_not_nil expectation
assert_equal [expectation], mock.__expectations__.to_a
assert_equal expectation.expectations, mock.__expectations__.to_a
end

def test_should_not_stub_everything_by_default
Expand Down Expand Up @@ -49,20 +49,20 @@ def test_should_be_equal
method_missing
singleton_method_undefined
initialize
Array
block_given?
].freeze

MACOS_EXCLUDED_METHODS =
MACOS && MACOS_VERSION >= MACOS_MOJAVE_VERSION ? [:syscall] : []

RUBY_V19_AND_LATER_EXCLUDED_METHODS = [
:object_id,
:method_missing,
:singleton_method_undefined,
:initialize,
:String,
:singleton_method_added,
*MACOS_EXCLUDED_METHODS
].freeze
RUBY_V19_AND_LATER_EXCLUDED_METHODS =
(PRE_RUBY_V19_EXCLUDED_METHODS.map(&:to_sym) + [
:object_id,
:String,
:singleton_method_added,
*MACOS_EXCLUDED_METHODS
]).freeze

OBJECT_METHODS = STANDARD_OBJECT_PUBLIC_INSTANCE_METHODS.reject do |m|
(m =~ /^__.*__$/) ||
Expand All @@ -87,28 +87,14 @@ def test_should_create_and_add_expectations
mock = build_mock
expectation1 = mock.expects(:method1)
expectation2 = mock.expects(:method2)
assert_equal [expectation1, expectation2].to_set, mock.__expectations__.to_set
end

def test_should_pass_backtrace_into_expectation
mock = build_mock
backtrace = Object.new
expectation = mock.expects(:method1, backtrace)
assert_equal backtrace, expectation.backtrace
end

def test_should_pass_backtrace_into_stub
mock = build_mock
backtrace = Object.new
stub = mock.stubs(:method1, backtrace)
assert_equal backtrace, stub.backtrace
assert_equal (expectation1.expectations + expectation2.expectations).to_set, mock.__expectations__.to_set
end

def test_should_create_and_add_stubs
mock = build_mock
stub1 = mock.stubs(:method1)
stub2 = mock.stubs(:method2)
assert_equal [stub1, stub2].to_set, mock.__expectations__.to_set
assert_equal (stub1.expectations + stub2.expectations).to_set, mock.__expectations__.to_set
end

def test_should_invoke_expectation_and_return_result
Expand Down
4 changes: 0 additions & 4 deletions test/unit/object_methods_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ def test_should_stub_self_for_object
assert_equal @object, @object.stubba_object
end

def test_nobody_expects_the_spanish_inquisition
assert_raises(Mocha::ExpectationErrorFactory.exception_class) { @object.expects(:the_spanish_inquisition) }
end

def test_should_alias_object_method
klass = Class.new { def self.method_x; end }
klass.extend(Mocha::ObjectMethods)
Expand Down