Merge pull request #681 from freerange/expectation-with-never-cardina…

…lity-should-display-deprecation-warning

Expectation with never cardinality should display deprecation warning

lib/mocha/cardinality.rb

@@ -34,6 +34,10 @@ def invocations_allowed?
       @invocations.size < maximum
     end
 
+    def invocations_never_allowed?
+      maximum.zero?
+    end
+
     def satisfied?
       @invocations.size >= required
     end

lib/mocha/expectation.rb

@@ -653,6 +653,11 @@ def invocations_allowed?
       @cardinality.invocations_allowed?
     end
 
+    # @private
+    def invocations_never_allowed?
+      @cardinality.invocations_never_allowed?
+    end
+
     # @private
     def satisfied?
       @cardinality.satisfied?

lib/mocha/expectation_list.rb

@@ -25,6 +25,10 @@ def match_allowing_invocation(invocation)
       matching_expectations(invocation).detect(&:invocations_allowed?)
     end
 
+    def match_never_allowing_invocation(invocation)
+      matching_expectations(invocation).detect(&:invocations_never_allowed?)
+    end
+
     def verified?(assertion_counter = nil)
       @expectations.all? { |expectation| expectation.verified?(assertion_counter) }
     end
@@ -49,8 +53,6 @@ def +(other)
       self.class.new(to_a + other.to_a)
     end
 
-    private
-
     def matching_expectations(invocation, ignoring_order: false)
       @expectations.select { |e| e.match?(invocation, ignoring_order: ignoring_order) }
     end

lib/mocha/mock.rb

@@ -8,6 +8,7 @@
 require 'mocha/parameters_matcher'
 require 'mocha/argument_iterator'
 require 'mocha/expectation_error_factory'
+require 'mocha/deprecation'
 
 module Mocha
   # Traditional mock object.
@@ -321,10 +322,21 @@ def handle_method_call(symbol, arguments, block)
       check_expiry
       check_responder_responds_to(symbol)
       invocation = Invocation.new(self, symbol, arguments, block)
-      if (matching_expectation_allowing_invocation = all_expectations.match_allowing_invocation(invocation))
+
+      matching_expectations = all_expectations.matching_expectations(invocation)
+      matching_expectation_allowing_invocation = matching_expectations.detect(&:invocations_allowed?)
+      matching_expectation_never_allowing_invocation = matching_expectations.detect(&:invocations_never_allowed?)
+
+      if matching_expectation_allowing_invocation
+        if matching_expectation_never_allowing_invocation
+          invocation_not_allowed_warning(invocation, matching_expectation_never_allowing_invocation)
+        end
         matching_expectation_allowing_invocation.invoke(invocation)
-      elsif (matching_expectation = all_expectations.match(invocation, ignoring_order: true)) || (!matching_expectation && !@everything_stubbed)
-        raise_unexpected_invocation_error(invocation, matching_expectation)
+      else
+        matching_expectation_ignoring_order = all_expectations.match(invocation, ignoring_order: true)
+        if matching_expectation_ignoring_order || (!matching_expectation_ignoring_order && !@everything_stubbed)
+          raise_unexpected_invocation_error(invocation, matching_expectation_ignoring_order)
+        end
       end
     end
 
@@ -369,6 +381,14 @@ def any_expectations?
 
     private
 
+    def invocation_not_allowed_warning(invocation, expectation)
+      messages = [
+        "The expectation defined at #{expectation.definition_location} does not allow invocations, but #{invocation.call_description} was invoked.",
+        'This invocation will cause the test to fail fast in a future version of Mocha.'
+      ]
+      Deprecation.warning(messages.join(' '))
+    end
+
     def raise_unexpected_invocation_error(invocation, matching_expectation)
       if @unexpected_invocation.nil?
         @unexpected_invocation = invocation

test/acceptance/mocked_methods_dispatch_test.rb

@@ -1,4 +1,6 @@
 require File.expand_path('../acceptance_test_helper', __FILE__)
+require 'deprecation_disabler'
+require 'execution_point'
 
 class MockedMethodDispatchTest < Mocha::TestCase
   include AcceptanceTest
@@ -72,4 +74,24 @@ def test_should_find_latest_expectation_with_range_of_expected_invocation_count_
     end
     assert_passed(test_result)
   end
+
+  def test_should_display_deprecation_warning_if_invocation_matches_expectation_with_never_cardinality
+    execution_point = nil
+    test_result = run_as_test do
+      mock = mock('mock')
+      mock.stubs(:method)
+      mock.expects(:method).never; execution_point = ExecutionPoint.current
+      DeprecationDisabler.disable_deprecations do
+        mock.method
+      end
+    end
+    assert_passed(test_result)
+    message = Mocha::Deprecation.messages.last
+    location = execution_point.location
+    expected = [
+      "The expectation defined at #{location} does not allow invocations, but #<Mock:mock>.method() was invoked.",
+      'This invocation will cause the test to fail fast in a future version of Mocha.'
+    ]
+    assert_equal expected.join(' '), message
+  end
 end

test/acceptance/parameter_matcher_test.rb

@@ -373,4 +373,15 @@ def test_should_not_match_parameters_when_values_do_not_add_up_to_ten
     end
     assert_failed(test_result)
   end
+
+  def test_should_only_call_matcher_block_once
+    test_result = run_as_test do
+      number_of_invocations = 0
+      mock = mock()
+      mock.stubs(:method).with { number_of_invocations += 1; true }
+      mock.method
+      assert_equal 1, number_of_invocations
+    end
+    assert_passed(test_result)
+  end
 end

test/unit/cardinality_test.rb

@@ -22,6 +22,13 @@ def test_should_allow_invocations_if_invocation_count_has_not_yet_reached_maximu
     assert !cardinality.invocations_allowed?
   end
 
+  def test_should_never_allow_invocations
+    cardinality = Cardinality.new.exactly(0)
+    assert cardinality.invocations_never_allowed?
+    cardinality << new_invocation
+    assert cardinality.invocations_never_allowed?
+  end
+
   def test_should_be_satisfied_if_invocations_so_far_have_reached_required_threshold
     cardinality = Cardinality.new(2, 3)
     assert !cardinality.satisfied?

test/unit/expectation_list_test.rb

@@ -69,6 +69,17 @@ def test_should_find_most_recent_matching_expectation_allowing_invocation
     assert_same expectation1, expectation_list.match_allowing_invocation(Invocation.new(:irrelevant, :my_method))
   end
 
+  def test_should_find_matching_expectation_never_allowing_invocation
+    expectation_list = ExpectationList.new
+    expectation1 = Expectation.new(nil, :my_method)
+    expectation2 = Expectation.new(nil, :my_method)
+    define_instance_method(expectation1, :invocations_never_allowed?) { true }
+    define_instance_method(expectation2, :invocations_never_allowed?) { false }
+    expectation_list.add(expectation1)
+    expectation_list.add(expectation2)
+    assert_same expectation1, expectation_list.match_never_allowing_invocation(Invocation.new(:irrelevant, :my_method))
+  end
+
   def test_should_combine_two_expectation_lists_into_one
     expectation_list1 = ExpectationList.new
     expectation_list2 = ExpectationList.new

test/unit/expectation_test.rb

@@ -88,6 +88,13 @@ def test_should_allow_invocations_until_expected_invocation_count_is_a_range_fro
     assert !expectation.invocations_allowed?
   end
 
+  def test_should_never_allow_invocations
+    expectation = new_expectation.never
+    assert expectation.invocations_never_allowed?
+    invoke(expectation)
+    assert expectation.invocations_never_allowed?
+  end
+
   def test_should_store_provided_backtrace
     backtrace = Object.new
     assert_equal backtrace, Expectation.new(nil, :expected_method, backtrace).backtrace