From 9d0d37a0a9b73d1c15d63a2896d19787b894a2a9 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Fri, 6 Mar 2020 11:18:03 +0000 Subject: [PATCH 1/3] Refactor: prep to turn class methods into instance ... in order to make a single Cardinality instance persist across updates to expectation cardinality. This include a 'times' method. To make space for it, the existing protected 'times' method has been renamed to 'count'. --- lib/mocha/cardinality.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index 703e0535c..ed23cab2b 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -62,21 +62,21 @@ def anticipated_times if allowed_any_number_of_times? 'allowed any number of times' elsif required.zero? && maximum.zero? - "expected #{times(maximum)}" + "expected #{count(maximum)}" elsif required == maximum - "expected exactly #{times(required)}" + "expected exactly #{count(required)}" elsif infinite?(maximum) - "expected at least #{times(required)}" + "expected at least #{count(required)}" elsif required.zero? - "expected at most #{times(maximum)}" + "expected at most #{count(maximum)}" else - "expected between #{required} and #{times(maximum)}" + "expected between #{required} and #{count(maximum)}" end end # rubocop:enable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity def invoked_times - "invoked #{times(@invocations.size)}" + "invoked #{count(@invocations.size)}" end def actual_invocations @@ -87,7 +87,7 @@ def actual_invocations attr_reader :required, :maximum - def times(number) + def count(number) case number when 0 then 'never' when 1 then 'once' From f7d901ccf5e467d5ad143d1e4ae3e532eb3006c0 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Fri, 6 Mar 2020 11:46:28 +0000 Subject: [PATCH 2/3] persist cardinality instance across updates Fixes #473. Earlier, whenever you qualified an Expectation with a cardinality, a new Cardinality instance would replace the old one. The moving of invocations recording to Cardinality in 59454a8 then meant that the invocations recorded prior to updating cardinality would get discarded along with the Cardinality instance, effectively being forgotten. We now update the quantifiers of the same Cardinality instance for each Expectation in order to persist the recording of invocations across updates to cardinality. --- lib/mocha/cardinality.rb | 43 ++++++++++--------- lib/mocha/expectation.rb | 14 +++--- .../expected_invocation_count_test.rb | 11 +++++ test/unit/cardinality_test.rb | 38 ++++++++-------- 4 files changed, 60 insertions(+), 46 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index ed23cab2b..ab9b4f38e 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -2,31 +2,28 @@ module Mocha class Cardinality INFINITY = 1 / 0.0 - class << self - def exactly(count) - new(count, count) - end + def initialize(required = 0, maximum = INFINITY) + update(required, maximum) + @invocations = [] + end - def at_least(count) - new(count, INFINITY) - end + def exactly(count) + update(count, count) + end - def at_most(count) - new(0, count) - end + def at_least(count) + update(count, INFINITY) + end - def times(range_or_count) - case range_or_count - when Range then new(range_or_count.first, range_or_count.last) - else new(range_or_count, range_or_count) - end - end + def at_most(count) + update(0, count) end - def initialize(required, maximum) - @required = required - @maximum = maximum - @invocations = [] + def times(range_or_count) + case range_or_count + when Range then update(range_or_count.first, range_or_count.last) + else update(range_or_count, range_or_count) + end end def <<(invocation) @@ -96,6 +93,12 @@ def count(number) end end + def update(required, maximum) + @required = required + @maximum = maximum + self + end + def infinite?(number) number.respond_to?(:infinite?) && number.infinite? end diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index 12b0f8ebe..d1c7f53c8 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -42,7 +42,7 @@ class Expectation # object.expected_method # # => verify fails def times(range) - @cardinality = Cardinality.times(range) + @cardinality.times(range) self end @@ -68,7 +68,7 @@ def times(range) # object.expected_method # # => verify fails def twice - @cardinality = Cardinality.exactly(2) + @cardinality.exactly(2) self end @@ -93,7 +93,7 @@ def twice # object.expects(:expected_method).once # # => verify fails def once - @cardinality = Cardinality.exactly(1) + @cardinality.exactly(1) self end @@ -110,7 +110,7 @@ def once # object.expects(:expected_method).never # # => verify succeeds def never - @cardinality = Cardinality.exactly(0) + @cardinality.exactly(0) self end @@ -130,7 +130,7 @@ def never # object.expected_method # # => verify fails def at_least(minimum_number_of_times) - @cardinality = Cardinality.at_least(minimum_number_of_times) + @cardinality.at_least(minimum_number_of_times) self end @@ -167,7 +167,7 @@ def at_least_once # object.expects(:expected_method).at_most(2) # 3.times { object.expected_method } # => unexpected invocation def at_most(maximum_number_of_times) - @cardinality = Cardinality.at_most(maximum_number_of_times) + @cardinality.at_most(maximum_number_of_times) self end @@ -549,7 +549,7 @@ def initialize(mock, expected_method_name, backtrace = nil) @block_matcher = BlockMatchers::OptionalBlock.new @ordering_constraints = [] @side_effects = [] - @cardinality = Cardinality.exactly(1) + @cardinality = Cardinality.new.exactly(1) @return_values = ReturnValues.new @yield_parameters = YieldParameters.new @backtrace = backtrace || caller diff --git a/test/acceptance/expected_invocation_count_test.rb b/test/acceptance/expected_invocation_count_test.rb index 558015ca9..836c561cc 100644 --- a/test/acceptance/expected_invocation_count_test.rb +++ b/test/acceptance/expected_invocation_count_test.rb @@ -226,4 +226,15 @@ def test_should_fail_fast_if_there_is_no_matching_expectation '- expected exactly once, invoked never: #.method(1)' ], test_result.failure_message_lines end + + def test_should_pass_if_cardinality_is_satisfied_using_calls_made_both_before_and_after_updating_cardinality + test_result = run_as_test do + mock = mock('mock') + expectation = mock.expects(:method) + mock.method + expectation.twice + mock.method + end + assert_passed(test_result) + end end diff --git a/test/unit/cardinality_test.rb b/test/unit/cardinality_test.rb index 885820abf..1c265e97a 100644 --- a/test/unit/cardinality_test.rb +++ b/test/unit/cardinality_test.rb @@ -34,39 +34,39 @@ def test_should_be_satisfied_if_invocations_so_far_have_reached_required_thresho end def test_should_describe_cardinality_defined_using_at_least - assert_equal 'allowed any number of times', Cardinality.at_least(0).anticipated_times - assert_equal 'expected at least once', Cardinality.at_least(1).anticipated_times - assert_equal 'expected at least twice', Cardinality.at_least(2).anticipated_times - assert_equal 'expected at least 3 times', Cardinality.at_least(3).anticipated_times + assert_equal 'allowed any number of times', Cardinality.new.at_least(0).anticipated_times + assert_equal 'expected at least once', Cardinality.new.at_least(1).anticipated_times + assert_equal 'expected at least twice', Cardinality.new.at_least(2).anticipated_times + assert_equal 'expected at least 3 times', Cardinality.new.at_least(3).anticipated_times end def test_should_describe_cardinality_defined_using_at_most - assert_equal 'expected at most once', Cardinality.at_most(1).anticipated_times - assert_equal 'expected at most twice', Cardinality.at_most(2).anticipated_times - assert_equal 'expected at most 3 times', Cardinality.at_most(3).anticipated_times + assert_equal 'expected at most once', Cardinality.new.at_most(1).anticipated_times + assert_equal 'expected at most twice', Cardinality.new.at_most(2).anticipated_times + assert_equal 'expected at most 3 times', Cardinality.new.at_most(3).anticipated_times end def test_should_describe_cardinality_defined_using_exactly - assert_equal 'expected never', Cardinality.exactly(0).anticipated_times - assert_equal 'expected exactly once', Cardinality.exactly(1).anticipated_times - assert_equal 'expected exactly twice', Cardinality.exactly(2).anticipated_times - assert_equal 'expected exactly 3 times', Cardinality.exactly(3).anticipated_times + assert_equal 'expected never', Cardinality.new.exactly(0).anticipated_times + assert_equal 'expected exactly once', Cardinality.new.exactly(1).anticipated_times + assert_equal 'expected exactly twice', Cardinality.new.exactly(2).anticipated_times + assert_equal 'expected exactly 3 times', Cardinality.new.exactly(3).anticipated_times end def test_should_describe_cardinality_defined_using_times_with_range - assert_equal 'expected between 2 and 4 times', Cardinality.times(2..4).anticipated_times - assert_equal 'expected between 1 and 3 times', Cardinality.times(1..3).anticipated_times + assert_equal 'expected between 2 and 4 times', Cardinality.new.times(2..4).anticipated_times + assert_equal 'expected between 1 and 3 times', Cardinality.new.times(1..3).anticipated_times end def test_should_need_verifying - assert Cardinality.exactly(2).needs_verifying? - assert Cardinality.at_least(3).needs_verifying? - assert Cardinality.at_most(2).needs_verifying? - assert Cardinality.times(4).needs_verifying? - assert Cardinality.times(2..4).needs_verifying? + assert Cardinality.new.exactly(2).needs_verifying? + assert Cardinality.new.at_least(3).needs_verifying? + assert Cardinality.new.at_most(2).needs_verifying? + assert Cardinality.new.times(4).needs_verifying? + assert Cardinality.new.times(2..4).needs_verifying? end def test_should_not_need_verifying - assert_equal false, Cardinality.at_least(0).needs_verifying? + assert_equal false, Cardinality.new.at_least(0).needs_verifying? end end From 5c876f4429ff673caa3c61dc8df43082b93a93b2 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Fri, 6 Mar 2020 12:14:32 +0000 Subject: [PATCH 3/3] Refactor: remove unnecessary return self The at_{least,most}_once methods simply delegate to at_least, and at_least returns self. So, ending with a call to at_least means we don't need to end with self in order to return self. --- lib/mocha/expectation.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index d1c7f53c8..447b59aad 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -149,7 +149,6 @@ def at_least(minimum_number_of_times) # # => verify fails def at_least_once at_least(1) - self end # Modifies expectation so that the expected method must be called at most a +maximum_number_of_times+. @@ -186,7 +185,6 @@ def at_most(maximum_number_of_times) # 2.times { object.expected_method } # => unexpected invocation def at_most_once at_most(1) - self end # Modifies expectation so that the expected method must be called with +expected_parameters+.