From 862b08e307ead49ccf2291d1f18066b6c7728a6b Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 19:29:48 +0000 Subject: [PATCH 01/15] Prepare to move invocation size based logic to Cardinality Cardinality and invocations go hand-in-hand. So, the target is to move invocations to Cardinality. --- lib/mocha/expectation.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index e8653e7d2..8a55b1a14 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -594,12 +594,7 @@ def inspect # @private def mocha_inspect message = "#{@cardinality.mocha_inspect}, " - message << case @invocations.size - when 0 then 'not yet invoked' - when 1 then 'invoked once' - when 2 then 'invoked twice' - else "invoked #{@invocations.size} times" - end + message << actual_invocations(@invocations.size) message << ': ' message << method_signature message << "; #{@ordering_constraints.map(&:mocha_inspect).join('; ')}" unless @ordering_constraints.empty? @@ -614,6 +609,19 @@ def method_signature private + def actual_invocations(invocation_count) + case invocation_count + when 0 then + 'not yet invoked' + when 1 then + 'invoked once' + when 2 then + 'invoked twice' + else + "invoked #{invocation_count} times" + end + end + def method_name "#{@mock.mocha_inspect}.#{@method_matcher.mocha_inspect}" end From 0f1478e2f284c7b867bf4154adeabffbd367e3c0 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 19:34:40 +0000 Subject: [PATCH 02/15] Move invocation size based logic to Cardinality Cardinality and invocations go hand-in-hand. So, the target is to move invocations to Cardinality. --- lib/mocha/cardinality.rb | 13 +++++++++++++ lib/mocha/expectation.rb | 15 +-------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index 6cc00684d..64ad07792 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -68,6 +68,19 @@ def mocha_inspect end end + def actual_invocations(invocation_count) + case invocation_count + when 0 then + 'not yet invoked' + when 1 then + 'invoked once' + when 2 then + 'invoked twice' + else + "invoked #{invocation_count} times" + end + end + protected attr_reader :required, :maximum diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index 8a55b1a14..5396835ca 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -594,7 +594,7 @@ def inspect # @private def mocha_inspect message = "#{@cardinality.mocha_inspect}, " - message << actual_invocations(@invocations.size) + message << @cardinality.actual_invocations(@invocations.size) message << ': ' message << method_signature message << "; #{@ordering_constraints.map(&:mocha_inspect).join('; ')}" unless @ordering_constraints.empty? @@ -609,19 +609,6 @@ def method_signature private - def actual_invocations(invocation_count) - case invocation_count - when 0 then - 'not yet invoked' - when 1 then - 'invoked once' - when 2 then - 'invoked twice' - else - "invoked #{invocation_count} times" - end - end - def method_name "#{@mock.mocha_inspect}.#{@method_matcher.mocha_inspect}" end From 59454a83648f76e46918779f7dd39a34927cff2c Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 19:46:11 +0000 Subject: [PATCH 03/15] Move invocations list to Cardinality The target is to encapsulate it inside Cardinality in a tell-don't-ask style. --- lib/mocha/cardinality.rb | 7 +++++++ lib/mocha/expectation.rb | 15 +++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index 64ad07792..ebe3cdd85 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -23,9 +23,16 @@ def times(range_or_count) end end + attr_reader :invocations + def initialize(required, maximum) @required = required @maximum = maximum + @invocations = [] + end + + def <<(invocation) + @invocations << invocation end def invocations_allowed?(invocation_count) diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index 5396835ca..c40882296 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -517,7 +517,6 @@ def initialize(mock, expected_method_name, backtrace = nil) @return_values = ReturnValues.new @yield_parameters = YieldParameters.new @backtrace = backtrace || caller - @invocations = [] end # @private @@ -557,31 +556,31 @@ def match?(actual_method_name, *actual_parameters) # @private def invocations_allowed? - @cardinality.invocations_allowed?(@invocations.size) + @cardinality.invocations_allowed?(@cardinality.invocations.size) end # @private def satisfied? - @cardinality.satisfied?(@invocations.size) + @cardinality.satisfied?(@cardinality.invocations.size) end # @private def invoke(*arguments) perform_side_effects invocation = Invocation.new(method_name, @yield_parameters, @return_values) - @invocations << invocation + @cardinality << invocation invocation.call(*arguments) { |*yield_args| yield(*yield_args) } end # @private def verified?(assertion_counter = nil) assertion_counter.increment if assertion_counter && @cardinality.needs_verifying? - @cardinality.verified?(@invocations.size) + @cardinality.verified?(@cardinality.invocations.size) end # @private def used? - @cardinality.used?(@invocations.size) + @cardinality.used?(@cardinality.invocations.size) end # @private @@ -594,7 +593,7 @@ def inspect # @private def mocha_inspect message = "#{@cardinality.mocha_inspect}, " - message << @cardinality.actual_invocations(@invocations.size) + message << @cardinality.actual_invocations(@cardinality.invocations.size) message << ': ' message << method_signature message << "; #{@ordering_constraints.map(&:mocha_inspect).join('; ')}" unless @ordering_constraints.empty? @@ -614,7 +613,7 @@ def method_name end def invocations - @invocations.map(&:mocha_inspect).join + @cardinality.invocations.map(&:mocha_inspect).join end end end From 836f85b8514c841781767c1d31e923af5074d4ae Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:03:39 +0000 Subject: [PATCH 04/15] Infer invocation count from invocations size The target is to encapsulate it inside Cardinality in a tell-don't-ask style. --- lib/mocha/cardinality.rb | 22 +++++++++++----------- lib/mocha/expectation.rb | 10 +++++----- test/unit/cardinality_test.rb | 29 +++++++++++++++++++++-------- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index ebe3cdd85..f2962f2fc 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -35,28 +35,28 @@ def <<(invocation) @invocations << invocation end - def invocations_allowed?(invocation_count) - invocation_count < maximum + def invocations_allowed? + @invocations.size < maximum end - def satisfied?(invocations_so_far) - invocations_so_far >= required + def satisfied? + @invocations.size >= required end def needs_verifying? !allowed_any_number_of_times? end - def verified?(invocation_count) - (invocation_count >= required) && (invocation_count <= maximum) + def verified? + (@invocations.size >= required) && (@invocations.size <= maximum) end def allowed_any_number_of_times? required.zero? && infinite?(maximum) end - def used?(invocation_count) - (invocation_count > 0) || maximum.zero? + def used? + @invocations.any? || maximum.zero? end def mocha_inspect @@ -75,8 +75,8 @@ def mocha_inspect end end - def actual_invocations(invocation_count) - case invocation_count + def actual_invocations + case @invocations.size when 0 then 'not yet invoked' when 1 then @@ -84,7 +84,7 @@ def actual_invocations(invocation_count) when 2 then 'invoked twice' else - "invoked #{invocation_count} times" + "invoked #{@invocations.size} times" end end diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index c40882296..a8fc4d49b 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -556,12 +556,12 @@ def match?(actual_method_name, *actual_parameters) # @private def invocations_allowed? - @cardinality.invocations_allowed?(@cardinality.invocations.size) + @cardinality.invocations_allowed? end # @private def satisfied? - @cardinality.satisfied?(@cardinality.invocations.size) + @cardinality.satisfied? end # @private @@ -575,12 +575,12 @@ def invoke(*arguments) # @private def verified?(assertion_counter = nil) assertion_counter.increment if assertion_counter && @cardinality.needs_verifying? - @cardinality.verified?(@cardinality.invocations.size) + @cardinality.verified? end # @private def used? - @cardinality.used?(@cardinality.invocations.size) + @cardinality.used? end # @private @@ -593,7 +593,7 @@ def inspect # @private def mocha_inspect message = "#{@cardinality.mocha_inspect}, " - message << @cardinality.actual_invocations(@cardinality.invocations.size) + message << @cardinality.actual_invocations message << ': ' message << method_signature message << "; #{@ordering_constraints.map(&:mocha_inspect).join('; ')}" unless @ordering_constraints.empty? diff --git a/test/unit/cardinality_test.rb b/test/unit/cardinality_test.rb index ee62c8014..ac5e39de5 100644 --- a/test/unit/cardinality_test.rb +++ b/test/unit/cardinality_test.rb @@ -1,23 +1,36 @@ require File.expand_path('../../test_helper', __FILE__) require 'mocha/cardinality' +require 'mocha/invocation' +require 'mocha/return_values' +require 'mocha/yield_parameters' class CardinalityTest < Mocha::TestCase include Mocha + def new_invocation + Invocation.new(:foo, YieldParameters.new, ReturnValues.new) + end + def test_should_allow_invocations_if_invocation_count_has_not_yet_reached_maximum cardinality = Cardinality.new(2, 3) - assert cardinality.invocations_allowed?(0) - assert cardinality.invocations_allowed?(1) - assert cardinality.invocations_allowed?(2) - assert !cardinality.invocations_allowed?(3) + assert cardinality.invocations_allowed? + cardinality << new_invocation + assert cardinality.invocations_allowed? + cardinality << new_invocation + assert cardinality.invocations_allowed? + cardinality << new_invocation + assert !cardinality.invocations_allowed? end def test_should_be_satisfied_if_invocations_so_far_have_reached_required_threshold cardinality = Cardinality.new(2, 3) - assert !cardinality.satisfied?(0) - assert !cardinality.satisfied?(1) - assert cardinality.satisfied?(2) - assert cardinality.satisfied?(3) + assert !cardinality.satisfied? + cardinality << new_invocation + assert !cardinality.satisfied? + cardinality << new_invocation + assert cardinality.satisfied? + cardinality << new_invocation + assert cardinality.satisfied? end def test_should_describe_cardinality From e58de2b8bb0141d514deee322482bf09de590b92 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:06:52 +0000 Subject: [PATCH 05/15] Encapsulate invocations inside Cardinality --- lib/mocha/cardinality.rb | 6 ++++-- lib/mocha/expectation.rb | 6 +----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index f2962f2fc..6efbed6d7 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -23,8 +23,6 @@ def times(range_or_count) end end - attr_reader :invocations - def initialize(required, maximum) @required = required @maximum = maximum @@ -88,6 +86,10 @@ def actual_invocations end end + def invocations + @invocations.map(&:mocha_inspect).join + end + protected attr_reader :required, :maximum diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index a8fc4d49b..c510e2957 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -597,7 +597,7 @@ def mocha_inspect message << ': ' message << method_signature message << "; #{@ordering_constraints.map(&:mocha_inspect).join('; ')}" unless @ordering_constraints.empty? - message << invocations if (ENV['MOCHA_OPTIONS'] || '').split(',').include?('verbose') + message << @cardinality.invocations if (ENV['MOCHA_OPTIONS'] || '').split(',').include?('verbose') message end @@ -611,9 +611,5 @@ def method_signature def method_name "#{@mock.mocha_inspect}.#{@method_matcher.mocha_inspect}" end - - def invocations - @cardinality.invocations.map(&:mocha_inspect).join - end end end From e20cb8a5f13926f43521f521a55e25586a4f5e15 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:10:49 +0000 Subject: [PATCH 06/15] Rename methods on Cardinality to better describe intention --- lib/mocha/cardinality.rb | 4 ++-- lib/mocha/expectation.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index 6efbed6d7..5c50da793 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -73,7 +73,7 @@ def mocha_inspect end end - def actual_invocations + def actual_times case @invocations.size when 0 then 'not yet invoked' @@ -86,7 +86,7 @@ def actual_invocations end end - def invocations + def actual_invocations @invocations.map(&:mocha_inspect).join end diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index c510e2957..b67ab0680 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -593,11 +593,11 @@ def inspect # @private def mocha_inspect message = "#{@cardinality.mocha_inspect}, " - message << @cardinality.actual_invocations + message << @cardinality.actual_times message << ': ' message << method_signature message << "; #{@ordering_constraints.map(&:mocha_inspect).join('; ')}" unless @ordering_constraints.empty? - message << @cardinality.invocations if (ENV['MOCHA_OPTIONS'] || '').split(',').include?('verbose') + message << @cardinality.actual_invocations if (ENV['MOCHA_OPTIONS'] || '').split(',').include?('verbose') message end From f87bf6c42f9646a9b20b0a5b50bbb170138dfc02 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:14:26 +0000 Subject: [PATCH 07/15] Rename methods on Cardinality to better describe intention --- lib/mocha/cardinality.rb | 2 +- lib/mocha/expectation.rb | 2 +- test/unit/cardinality_test.rb | 26 +++++++++++++------------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index 5c50da793..972132ecc 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -57,7 +57,7 @@ def used? @invocations.any? || maximum.zero? end - def mocha_inspect + def expected_times if allowed_any_number_of_times? 'allowed any number of times' elsif required.zero? && maximum.zero? diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index b67ab0680..31b8a15b4 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -592,7 +592,7 @@ def inspect # @private def mocha_inspect - message = "#{@cardinality.mocha_inspect}, " + message = "#{@cardinality.expected_times}, " message << @cardinality.actual_times message << ': ' message << method_signature diff --git a/test/unit/cardinality_test.rb b/test/unit/cardinality_test.rb index ac5e39de5..d83f8e4da 100644 --- a/test/unit/cardinality_test.rb +++ b/test/unit/cardinality_test.rb @@ -34,23 +34,23 @@ def test_should_be_satisfied_if_invocations_so_far_have_reached_required_thresho end def test_should_describe_cardinality - assert_equal 'allowed any number of times', Cardinality.at_least(0).mocha_inspect + assert_equal 'allowed any number of times', Cardinality.at_least(0).expected_times - assert_equal 'expected at most once', Cardinality.at_most(1).mocha_inspect - assert_equal 'expected at most twice', Cardinality.at_most(2).mocha_inspect - assert_equal 'expected at most 3 times', Cardinality.at_most(3).mocha_inspect + assert_equal 'expected at most once', Cardinality.at_most(1).expected_times + assert_equal 'expected at most twice', Cardinality.at_most(2).expected_times + assert_equal 'expected at most 3 times', Cardinality.at_most(3).expected_times - assert_equal 'expected at least once', Cardinality.at_least(1).mocha_inspect - assert_equal 'expected at least twice', Cardinality.at_least(2).mocha_inspect - assert_equal 'expected at least 3 times', Cardinality.at_least(3).mocha_inspect + assert_equal 'expected at least once', Cardinality.at_least(1).expected_times + assert_equal 'expected at least twice', Cardinality.at_least(2).expected_times + assert_equal 'expected at least 3 times', Cardinality.at_least(3).expected_times - assert_equal 'expected never', Cardinality.exactly(0).mocha_inspect - assert_equal 'expected exactly once', Cardinality.exactly(1).mocha_inspect - assert_equal 'expected exactly twice', Cardinality.exactly(2).mocha_inspect - assert_equal 'expected exactly 3 times', Cardinality.times(3).mocha_inspect + assert_equal 'expected never', Cardinality.exactly(0).expected_times + assert_equal 'expected exactly once', Cardinality.exactly(1).expected_times + assert_equal 'expected exactly twice', Cardinality.exactly(2).expected_times + assert_equal 'expected exactly 3 times', Cardinality.times(3).expected_times - assert_equal 'expected between 2 and 4 times', Cardinality.times(2..4).mocha_inspect - assert_equal 'expected between 1 and 3 times', Cardinality.times(1..3).mocha_inspect + assert_equal 'expected between 2 and 4 times', Cardinality.times(2..4).expected_times + assert_equal 'expected between 1 and 3 times', Cardinality.times(1..3).expected_times end def test_should_need_verifying From aa27319f18a95d6b40bd95644380caf0336b7b4c Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:18:29 +0000 Subject: [PATCH 08/15] Rename methods on Cardinality to better describe intention expected hadn't quite sounded right because it also included allowed. --- lib/mocha/cardinality.rb | 4 ++-- lib/mocha/expectation.rb | 4 ++-- test/unit/cardinality_test.rb | 26 +++++++++++++------------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index 972132ecc..cbf83af3e 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -57,7 +57,7 @@ def used? @invocations.any? || maximum.zero? end - def expected_times + def anticipated_times if allowed_any_number_of_times? 'allowed any number of times' elsif required.zero? && maximum.zero? @@ -73,7 +73,7 @@ def expected_times end end - def actual_times + def invoked_times case @invocations.size when 0 then 'not yet invoked' diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index 31b8a15b4..ee5ee02c0 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -592,8 +592,8 @@ def inspect # @private def mocha_inspect - message = "#{@cardinality.expected_times}, " - message << @cardinality.actual_times + message = "#{@cardinality.anticipated_times}, " + message << @cardinality.invoked_times message << ': ' message << method_signature message << "; #{@ordering_constraints.map(&:mocha_inspect).join('; ')}" unless @ordering_constraints.empty? diff --git a/test/unit/cardinality_test.rb b/test/unit/cardinality_test.rb index d83f8e4da..df6f15479 100644 --- a/test/unit/cardinality_test.rb +++ b/test/unit/cardinality_test.rb @@ -34,23 +34,23 @@ def test_should_be_satisfied_if_invocations_so_far_have_reached_required_thresho end def test_should_describe_cardinality - assert_equal 'allowed any number of times', Cardinality.at_least(0).expected_times + assert_equal 'allowed any number of times', Cardinality.at_least(0).anticipated_times - assert_equal 'expected at most once', Cardinality.at_most(1).expected_times - assert_equal 'expected at most twice', Cardinality.at_most(2).expected_times - assert_equal 'expected at most 3 times', Cardinality.at_most(3).expected_times + 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 least once', Cardinality.at_least(1).expected_times - assert_equal 'expected at least twice', Cardinality.at_least(2).expected_times - assert_equal 'expected at least 3 times', Cardinality.at_least(3).expected_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 'expected never', Cardinality.exactly(0).expected_times - assert_equal 'expected exactly once', Cardinality.exactly(1).expected_times - assert_equal 'expected exactly twice', Cardinality.exactly(2).expected_times - assert_equal 'expected exactly 3 times', Cardinality.times(3).expected_times + 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.times(3).anticipated_times - assert_equal 'expected between 2 and 4 times', Cardinality.times(2..4).expected_times - assert_equal 'expected between 1 and 3 times', Cardinality.times(1..3).expected_times + 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 end def test_should_need_verifying From b6d10dc543fb7732f8ca819da31546c4f9bca4cd Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:21:20 +0000 Subject: [PATCH 09/15] Interpolate vs append simple expressions in Expectation#mocha_inspect --- lib/mocha/expectation.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb index ee5ee02c0..a0ca2b33b 100644 --- a/lib/mocha/expectation.rb +++ b/lib/mocha/expectation.rb @@ -592,10 +592,7 @@ def inspect # @private def mocha_inspect - message = "#{@cardinality.anticipated_times}, " - message << @cardinality.invoked_times - message << ': ' - message << method_signature + message = "#{@cardinality.anticipated_times}, #{@cardinality.invoked_times}: #{method_signature}" message << "; #{@ordering_constraints.map(&:mocha_inspect).join('; ')}" unless @ordering_constraints.empty? message << @cardinality.actual_invocations if (ENV['MOCHA_OPTIONS'] || '').split(',').include?('verbose') message From 4a311184f94ace902490d3f588631327dbe55f1b Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:32:21 +0000 Subject: [PATCH 10/15] Use same times logic for non-zero configured & invoked --- lib/mocha/cardinality.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index cbf83af3e..8f6f868af 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -77,12 +77,8 @@ def invoked_times case @invocations.size when 0 then 'not yet invoked' - when 1 then - 'invoked once' - when 2 then - 'invoked twice' else - "invoked #{@invocations.size} times" + "invoked #{times(@invocations.size)}" end end From 09742f1829f4db7c1b27e7c7e202cf5b8c66e2f3 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:38:27 +0000 Subject: [PATCH 11/15] Cardinality#times is never invoked with 0 --- lib/mocha/cardinality.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index 8f6f868af..f0e0644e0 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -92,7 +92,6 @@ def actual_invocations def times(number) case number - when 0 then 'no times' when 1 then 'once' when 2 then 'twice' else "#{number} times" From 54f38b079e726ff44f9d4dcde9ebe7c81a1f2c13 Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 20:41:26 +0000 Subject: [PATCH 12/15] Simplify Cardinality#invoked_times Using more expressive & concise conditionals & booleans. --- lib/mocha/cardinality.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index f0e0644e0..4df969f37 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -74,12 +74,7 @@ def anticipated_times end def invoked_times - case @invocations.size - when 0 then - 'not yet invoked' - else - "invoked #{times(@invocations.size)}" - end + @invocations.none? ? 'not yet invoked' : "invoked #{times(@invocations.size)}" end def actual_invocations From 290f390ebdd5398c6188671d213317d96fe2316a Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Wed, 30 Oct 2019 21:24:38 +0000 Subject: [PATCH 13/15] Use consistent phrasing for configured & invoked times --- lib/mocha/cardinality.rb | 5 +++-- test/acceptance/exception_rescue_test.rb | 2 +- test/acceptance/expected_invocation_count_test.rb | 4 ++-- .../stub_any_instance_method_defined_on_superclass_test.rb | 2 +- .../stub_class_method_defined_on_superclass_test.rb | 2 +- test/integration/shared_tests.rb | 6 +++--- test/unit/expectation_test.rb | 2 +- 7 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/mocha/cardinality.rb b/lib/mocha/cardinality.rb index 4df969f37..6a69e662c 100644 --- a/lib/mocha/cardinality.rb +++ b/lib/mocha/cardinality.rb @@ -61,7 +61,7 @@ def anticipated_times if allowed_any_number_of_times? 'allowed any number of times' elsif required.zero? && maximum.zero? - 'expected never' + "expected #{times(maximum)}" elsif required == maximum "expected exactly #{times(required)}" elsif infinite?(maximum) @@ -74,7 +74,7 @@ def anticipated_times end def invoked_times - @invocations.none? ? 'not yet invoked' : "invoked #{times(@invocations.size)}" + "invoked #{times(@invocations.size)}" end def actual_invocations @@ -87,6 +87,7 @@ def actual_invocations def times(number) case number + when 0 then 'never' when 1 then 'once' when 2 then 'twice' else "#{number} times" diff --git a/test/acceptance/exception_rescue_test.rb b/test/acceptance/exception_rescue_test.rb index e8f4a5608..54905503a 100644 --- a/test/acceptance/exception_rescue_test.rb +++ b/test/acceptance/exception_rescue_test.rb @@ -48,7 +48,7 @@ def test_unsatisfied_expectation_exception_is_not_caught_by_standard_rescue assert_equal [ 'not all expectations were satisfied', 'unsatisfied expectations:', - '- expected exactly once, not yet invoked: #.some_method(any_parameters)' + '- expected exactly once, invoked never: #.some_method(any_parameters)' ], test_result.failure_message_lines end end diff --git a/test/acceptance/expected_invocation_count_test.rb b/test/acceptance/expected_invocation_count_test.rb index fc177709a..9067df6a6 100644 --- a/test/acceptance/expected_invocation_count_test.rb +++ b/test/acceptance/expected_invocation_count_test.rb @@ -155,7 +155,7 @@ def test_should_fail_if_method_is_expected_at_least_once_but_is_never_called assert_equal [ 'not all expectations were satisfied', 'unsatisfied expectations:', - '- expected at least once, not yet invoked: #.method(any_parameters)' + '- expected at least once, invoked never: #.method(any_parameters)' ], test_result.failure_message_lines end @@ -224,7 +224,7 @@ def test_should_fail_fast_if_there_is_no_matching_expectation assert_equal [ 'unexpected invocation: #.method()', 'unsatisfied expectations:', - '- expected exactly once, not yet invoked: #.method(1)' + '- expected exactly once, invoked never: #.method(1)' ], test_result.failure_message_lines end end diff --git a/test/acceptance/stub_any_instance_method_defined_on_superclass_test.rb b/test/acceptance/stub_any_instance_method_defined_on_superclass_test.rb index 27152c6d0..8087e1c11 100644 --- a/test/acceptance/stub_any_instance_method_defined_on_superclass_test.rb +++ b/test/acceptance/stub_any_instance_method_defined_on_superclass_test.rb @@ -59,7 +59,7 @@ def my_instance_method; end assert_equal [ 'not all expectations were satisfied', 'unsatisfied expectations:', - '- expected exactly once, not yet invoked: #.my_instance_method(any_parameters)' + '- expected exactly once, invoked never: #.my_instance_method(any_parameters)' ], test_result.failure_message_lines end end diff --git a/test/acceptance/stub_class_method_defined_on_superclass_test.rb b/test/acceptance/stub_class_method_defined_on_superclass_test.rb index 14797c65c..6b4a74f5f 100644 --- a/test/acceptance/stub_class_method_defined_on_superclass_test.rb +++ b/test/acceptance/stub_class_method_defined_on_superclass_test.rb @@ -138,7 +138,7 @@ def self.my_class_method; end assert_equal [ 'not all expectations were satisfied', 'unsatisfied expectations:', - '- expected exactly once, not yet invoked: superklass.my_class_method(any_parameters)' + '- expected exactly once, invoked never: superklass.my_class_method(any_parameters)' ], test_result.failure_message_lines end # rubocop:enable Lint/DuplicateMethods diff --git a/test/integration/shared_tests.rb b/test/integration/shared_tests.rb index 00ea88292..f5ef1886f 100644 --- a/test/integration/shared_tests.rb +++ b/test/integration/shared_tests.rb @@ -63,7 +63,7 @@ def test_mock_object_unsatisfied_expectation assert_equal [ 'not all expectations were satisfied', 'unsatisfied expectations:', - '- expected exactly once, not yet invoked: #.expected(any_parameters)' + '- expected exactly once, invoked never: #.expected(any_parameters)' ], test_result.failure_message_lines end @@ -101,7 +101,7 @@ def test_mock_object_unsatisfied_expectation_in_setup assert_equal [ 'not all expectations were satisfied', 'unsatisfied expectations:', - '- expected exactly once, not yet invoked: #.expected(any_parameters)' + '- expected exactly once, invoked never: #.expected(any_parameters)' ], test_result.failure_message_lines end @@ -151,7 +151,7 @@ def test_real_object_unsatisfied_expectation assert_equal [ 'not all expectations were satisfied', 'unsatisfied expectations:', - "- expected exactly once, not yet invoked: #{object.mocha_inspect}.expected(any_parameters)" + "- expected exactly once, invoked never: #{object.mocha_inspect}.expected(any_parameters)" ], test_result.failure_message_lines end diff --git a/test/unit/expectation_test.rb b/test/unit/expectation_test.rb index f0c4ed459..6e7369370 100644 --- a/test/unit/expectation_test.rb +++ b/test/unit/expectation_test.rb @@ -282,7 +282,7 @@ def test_should_verify_successfully_if_expected_call_was_made_at_least_once def test_should_not_verify_successfully_if_expected_call_was_not_made_at_least_once expectation = new_expectation.with(1, 2, 3).at_least_once assert !expectation.verified? - assert_match(/expected at least once, not yet invoked/i, expectation.mocha_inspect) + assert_match(/expected at least once, invoked never/i, expectation.mocha_inspect) end def test_should_verify_successfully_if_expected_call_was_made_expected_number_of_times From ad0fa3c12769a2c16bf6d4619308a1619c85524d Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Mon, 4 Nov 2019 21:13:46 +0000 Subject: [PATCH 14/15] Reduce test's knowledge of irrelevant details --- lib/mocha/invocation.rb | 4 +++- test/unit/cardinality_test.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/mocha/invocation.rb b/lib/mocha/invocation.rb index 03568a58b..d16ad772a 100644 --- a/lib/mocha/invocation.rb +++ b/lib/mocha/invocation.rb @@ -1,9 +1,11 @@ require 'mocha/parameters_matcher' require 'mocha/raised_exception' +require 'mocha/return_values' +require 'mocha/yield_parameters' module Mocha class Invocation - def initialize(method_name, yield_parameters, return_values) + def initialize(method_name, yield_parameters = YieldParameters.new, return_values = ReturnValues.new) @method_name = method_name @yield_parameters = yield_parameters @return_values = return_values diff --git a/test/unit/cardinality_test.rb b/test/unit/cardinality_test.rb index df6f15479..048115dec 100644 --- a/test/unit/cardinality_test.rb +++ b/test/unit/cardinality_test.rb @@ -8,7 +8,7 @@ class CardinalityTest < Mocha::TestCase include Mocha def new_invocation - Invocation.new(:foo, YieldParameters.new, ReturnValues.new) + Invocation.new(:irrelevant) end def test_should_allow_invocations_if_invocation_count_has_not_yet_reached_maximum From 9e449b0c79d41d7045c49a1af648dfe89edcf31e Mon Sep 17 00:00:00 2001 From: Nitish Rathi Date: Tue, 5 Nov 2019 10:47:45 +0000 Subject: [PATCH 15/15] Record result of invocation before 'returning' it This helps us avoid rescuing an exception raised by code elsewhere in the Mocha code. Also, enables us to handle the missed scenario where Expectaction#throws is called, 'coz without it, we'd need a generic way to catch any thrown tag and that might be quite awkward. --- lib/mocha/exception_raiser.rb | 3 +- lib/mocha/invocation.rb | 19 ++++++-- lib/mocha/return_values.rb | 6 +-- lib/mocha/single_return_value.rb | 3 +- lib/mocha/thrower.rb | 3 +- lib/mocha/thrown_object.rb | 12 +++++ ...invocations_alongside_expectations_test.rb | 16 ++++--- test/unit/exception_raiser_test.rb | 15 ++++--- test/unit/return_values_test.rb | 45 ++++++++++--------- test/unit/single_return_value_test.rb | 7 ++- test/unit/thrower_test.rb | 9 +++- 11 files changed, 94 insertions(+), 44 deletions(-) create mode 100644 lib/mocha/thrown_object.rb diff --git a/lib/mocha/exception_raiser.rb b/lib/mocha/exception_raiser.rb index 47f43a577..8b2430dfb 100644 --- a/lib/mocha/exception_raiser.rb +++ b/lib/mocha/exception_raiser.rb @@ -5,7 +5,8 @@ def initialize(exception, message) @message = message end - def evaluate + def evaluate(invocation) + invocation.raised(@exception) raise @exception, @exception.to_s if @exception.is_a?(Module) && (@exception < Interrupt) raise @exception, @message if @message raise @exception diff --git a/lib/mocha/invocation.rb b/lib/mocha/invocation.rb index d16ad772a..15a653eab 100644 --- a/lib/mocha/invocation.rb +++ b/lib/mocha/invocation.rb @@ -1,6 +1,7 @@ require 'mocha/parameters_matcher' require 'mocha/raised_exception' require 'mocha/return_values' +require 'mocha/thrown_object' require 'mocha/yield_parameters' module Mocha @@ -10,6 +11,7 @@ def initialize(method_name, yield_parameters = YieldParameters.new, return_value @yield_parameters = yield_parameters @return_values = return_values @yields = [] + @result = nil end def call(*arguments) @@ -18,10 +20,19 @@ def call(*arguments) @yields << ParametersMatcher.new(yield_parameters) yield(*yield_parameters) end - @result = @return_values.next - rescue Exception => e # rubocop:disable Lint/RescueException - @result = RaisedException.new(e) - raise + @return_values.next(self) + end + + def returned(value) + @result = value + end + + def raised(exception) + @result = RaisedException.new(exception) + end + + def threw(tag, value) + @result = ThrownObject.new(tag, value) end def mocha_inspect diff --git a/lib/mocha/return_values.rb b/lib/mocha/return_values.rb index c7a0263a8..8479e87a5 100644 --- a/lib/mocha/return_values.rb +++ b/lib/mocha/return_values.rb @@ -12,11 +12,11 @@ def initialize(*values) @values = values end - def next + def next(invocation) case @values.length when 0 then nil - when 1 then @values.first.evaluate - else @values.shift.evaluate + when 1 then @values.first.evaluate(invocation) + else @values.shift.evaluate(invocation) end end diff --git a/lib/mocha/single_return_value.rb b/lib/mocha/single_return_value.rb index 42535274e..b7145c79a 100644 --- a/lib/mocha/single_return_value.rb +++ b/lib/mocha/single_return_value.rb @@ -6,7 +6,8 @@ def initialize(value) @value = value end - def evaluate + def evaluate(invocation) + invocation.returned(@value) @value end end diff --git a/lib/mocha/thrower.rb b/lib/mocha/thrower.rb index c51e0ed00..f3e4bc710 100644 --- a/lib/mocha/thrower.rb +++ b/lib/mocha/thrower.rb @@ -5,7 +5,8 @@ def initialize(tag, object = nil) @object = object end - def evaluate + def evaluate(invocation) + invocation.threw(@tag, @object) throw @tag, @object end end diff --git a/lib/mocha/thrown_object.rb b/lib/mocha/thrown_object.rb new file mode 100644 index 000000000..254b3dedf --- /dev/null +++ b/lib/mocha/thrown_object.rb @@ -0,0 +1,12 @@ +module Mocha + class ThrownObject + def initialize(tag, value = nil) + @tag = tag + @value = value + end + + def mocha_inspect + "threw (#{@tag.mocha_inspect}, #{@value.mocha_inspect})" + end + end +end diff --git a/test/acceptance/display_matching_invocations_alongside_expectations_test.rb b/test/acceptance/display_matching_invocations_alongside_expectations_test.rb index 236da47ed..9c3e4de8c 100644 --- a/test/acceptance/display_matching_invocations_alongside_expectations_test.rb +++ b/test/acceptance/display_matching_invocations_alongside_expectations_test.rb @@ -19,16 +19,18 @@ def test_should_display_results test_result = run_as_test do foo = mock('foo') foo.expects(:bar).with(1).returns('a') - foo.stubs(:bar).with(any_parameters).returns('f').raises(StandardError) + foo.stubs(:bar).with(any_parameters).returns('f').raises(StandardError).throws(:tag, 'value') foo.bar(1, 2) assert_raise(StandardError) { foo.bar(3, 4) } + assert_throws(:tag) { foo.bar(5, 6) } end assert_invocations( test_result, - '- allowed any number of times, invoked twice: #.bar(any_parameters)', + '- allowed any number of times, invoked 3 times: #.bar(any_parameters)', ' - #.bar(1, 2) # => "f"', - ' - #.bar(3, 4) # => raised StandardError' + ' - #.bar(3, 4) # => raised StandardError', + ' - #.bar(5, 6) # => threw (:tag, "value")' ) end @@ -36,16 +38,18 @@ def test_should_display_yields test_result = run_as_test do foo = mock('foo') foo.expects(:bar).with(1).returns('a') - foo.stubs(:bar).with(any_parameters).multiple_yields(%w[b c], %w[d e]).returns('f').raises(StandardError) + foo.stubs(:bar).with(any_parameters).multiple_yields(%w[b c], %w[d e]).returns('f').raises(StandardError).throws(:tag, 'value') foo.bar(1, 2) { |_ignored| } assert_raise(StandardError) { foo.bar(3, 4) { |_ignored| } } + assert_throws(:tag) { foo.bar(5, 6) { |_ignored| } } end assert_invocations( test_result, - '- allowed any number of times, invoked twice: #.bar(any_parameters)', + '- allowed any number of times, invoked 3 times: #.bar(any_parameters)', ' - #.bar(1, 2) # => "f" after yielding ("b", "c"), then ("d", "e")', - ' - #.bar(3, 4) # => raised StandardError after yielding ("b", "c"), then ("d", "e")' + ' - #.bar(3, 4) # => raised StandardError after yielding ("b", "c"), then ("d", "e")', + ' - #.bar(5, 6) # => threw (:tag, "value") after yielding ("b", "c"), then ("d", "e")' ) end diff --git a/test/unit/exception_raiser_test.rb b/test/unit/exception_raiser_test.rb index bc481905f..a3f42314b 100644 --- a/test/unit/exception_raiser_test.rb +++ b/test/unit/exception_raiser_test.rb @@ -1,40 +1,45 @@ require File.expand_path('../../test_helper', __FILE__) +require 'mocha/invocation' require 'mocha/exception_raiser' require 'timeout' class ExceptionRaiserTest < Mocha::TestCase include Mocha + def new_invocation + Invocation.new(:irrelevant) + end + def test_should_raise_exception_with_specified_class_and_default_message exception_class = Class.new(StandardError) raiser = ExceptionRaiser.new(exception_class, nil) - exception = assert_raises(exception_class) { raiser.evaluate } + exception = assert_raises(exception_class) { raiser.evaluate(new_invocation) } assert_equal exception_class.to_s, exception.message end def test_should_raise_exception_with_specified_class_and_message exception_class = Class.new(StandardError) raiser = ExceptionRaiser.new(exception_class, 'message') - exception = assert_raises(exception_class) { raiser.evaluate } + exception = assert_raises(exception_class) { raiser.evaluate(new_invocation) } assert_equal 'message', exception.message end def test_should_raise_exception_instance exception_class = Class.new(StandardError) raiser = ExceptionRaiser.new(exception_class.new('message'), nil) - exception = assert_raises(exception_class) { raiser.evaluate } + exception = assert_raises(exception_class) { raiser.evaluate(new_invocation) } assert_equal 'message', exception.message end def test_should_raise_interrupt_exception_with_default_message_so_it_works_in_ruby_1_8_6 raiser = ExceptionRaiser.new(Interrupt, nil) - assert_raises(Interrupt) { raiser.evaluate } + assert_raises(Interrupt) { raiser.evaluate(new_invocation) } end def test_should_raise_subclass_of_interrupt_exception_with_default_message_so_it_works_in_ruby_1_8_6 exception_class = Class.new(Interrupt) raiser = ExceptionRaiser.new(exception_class, nil) - assert_raises(exception_class) { raiser.evaluate } + assert_raises(exception_class) { raiser.evaluate(new_invocation) } end end diff --git a/test/unit/return_values_test.rb b/test/unit/return_values_test.rb index 375427b32..c43aaca61 100644 --- a/test/unit/return_values_test.rb +++ b/test/unit/return_values_test.rb @@ -1,61 +1,66 @@ require File.expand_path('../../test_helper', __FILE__) +require 'mocha/invocation' require 'mocha/return_values' class ReturnValuesTest < Mocha::TestCase include Mocha + def new_invocation + Invocation.new(:irrelevant) + end + def test_should_return_nil values = ReturnValues.new - assert_nil values.next + assert_nil values.next(new_invocation) end def test_should_keep_returning_nil values = ReturnValues.new - values.next - assert_nil values.next - assert_nil values.next + values.next(new_invocation) + assert_nil values.next(new_invocation) + assert_nil values.next(new_invocation) end def test_should_return_evaluated_single_return_value values = ReturnValues.new(SingleReturnValue.new('value')) - assert_equal 'value', values.next + assert_equal 'value', values.next(new_invocation) end def test_should_keep_returning_evaluated_single_return_value values = ReturnValues.new(SingleReturnValue.new('value')) - values.next - assert_equal 'value', values.next - assert_equal 'value', values.next + values.next(new_invocation) + assert_equal 'value', values.next(new_invocation) + assert_equal 'value', values.next(new_invocation) end def test_should_return_consecutive_evaluated_single_return_values values = ReturnValues.new(SingleReturnValue.new('value_1'), SingleReturnValue.new('value_2')) - assert_equal 'value_1', values.next - assert_equal 'value_2', values.next + assert_equal 'value_1', values.next(new_invocation) + assert_equal 'value_2', values.next(new_invocation) end def test_should_keep_returning_last_of_consecutive_evaluated_single_return_values values = ReturnValues.new(SingleReturnValue.new('value_1'), SingleReturnValue.new('value_2')) - values.next - values.next - assert_equal 'value_2', values.next - assert_equal 'value_2', values.next + values.next(new_invocation) + values.next(new_invocation) + assert_equal 'value_2', values.next(new_invocation) + assert_equal 'value_2', values.next(new_invocation) end def test_should_build_single_return_values_for_each_values values = ReturnValues.build('value_1', 'value_2', 'value_3').values - assert_equal 'value_1', values[0].evaluate - assert_equal 'value_2', values[1].evaluate - assert_equal 'value_3', values[2].evaluate + assert_equal 'value_1', values[0].evaluate(new_invocation) + assert_equal 'value_2', values[1].evaluate(new_invocation) + assert_equal 'value_3', values[2].evaluate(new_invocation) end def test_should_combine_two_sets_of_return_values values1 = ReturnValues.build('value_1') values2 = ReturnValues.build('value_2a', 'value_2b') values = (values1 + values2).values - assert_equal 'value_1', values[0].evaluate - assert_equal 'value_2a', values[1].evaluate - assert_equal 'value_2b', values[2].evaluate + assert_equal 'value_1', values[0].evaluate(new_invocation) + assert_equal 'value_2a', values[1].evaluate(new_invocation) + assert_equal 'value_2b', values[2].evaluate(new_invocation) end end diff --git a/test/unit/single_return_value_test.rb b/test/unit/single_return_value_test.rb index 6941f91bb..6e7153c19 100644 --- a/test/unit/single_return_value_test.rb +++ b/test/unit/single_return_value_test.rb @@ -1,12 +1,17 @@ require File.expand_path('../../test_helper', __FILE__) +require 'mocha/invocation' require 'mocha/single_return_value' class SingleReturnValueTest < Mocha::TestCase include Mocha + def new_invocation + Invocation.new(:irrelevant) + end + def test_should_return_value value = SingleReturnValue.new('value') - assert_equal 'value', value.evaluate + assert_equal 'value', value.evaluate(new_invocation) end end diff --git a/test/unit/thrower_test.rb b/test/unit/thrower_test.rb index 2a9572302..cf1b4786c 100644 --- a/test/unit/thrower_test.rb +++ b/test/unit/thrower_test.rb @@ -1,18 +1,23 @@ require File.expand_path('../../test_helper', __FILE__) +require 'mocha/invocation' require 'mocha/thrower' class ThrowerTest < Mocha::TestCase include Mocha + def new_invocation + Invocation.new(:irrelevant) + end + def test_should_throw_tag thrower = Thrower.new(:tag) - assert_throws(:tag) { thrower.evaluate } + assert_throws(:tag) { thrower.evaluate(new_invocation) } end def test_should_throw_tag_with_return_value thrower = Thrower.new(:tag, 'return-value') - return_value = catch(:tag) { thrower.evaluate } + return_value = catch(:tag) { thrower.evaluate(new_invocation) } assert_equal 'return-value', return_value end end