Fail fast if invocation matches never expectation

[floehopper]

TODO

• Come up with a version of this that emits a deprecation warning rather than actually changing the behaviour.
• Ensure all the relevant documentation is updated, e.g. method dispatch in README, class docs for Mock, and docs for any relevant methods.
• Update commit note(s) to reference being consistent with the behaviour described in the following test which was added in d358377

mocha/test/acceptance/expected_invocation_count_test.rb

Lines 202 to 214 in f899c03

	def test_should_fail_fast_if_method_is_never_expected_but_is_called_once_even_if_everything_is_stubbed
	test_result = run_as_test do
	stub = stub_everything('stub')
	stub.expects(:method).never
	1.times { stub.method }
	end
	assert_failed(test_result)
	assert_equal [
	'unexpected invocation: #<Mock:stub>.method()',
	'unsatisfied expectations:',
	'- expected never, invoked once: #<Mock:stub>.method(any_parameters)'
	], test_result.failure_message_lines
	end

Previously when an invocation matched an expectation which did not allow invocations (i.e. Expectation#never had been called on it), but the invocation also matched another expectation which did allow invocations, then the test would not fail with an unexpected invocation error.

This was happening because neither the if condition was true, because the "never" expectation was not returned by ExpectationList#match_allowing_invocation, but the other expectation allowing expectations was returned. Thus Expectation#invoke was called on the latter and Mock#raise_unexpected_invocation_error was not called.

This behaviour was confusing and had led to a number of issues being raised over the years: #44, #131, #490 & most recently #678. Previously I had thought this was a symptom of the JMock v1 dispatch behaviour (which might still be true) and thus would be best addressed by adopting the JMock v2 dispatch behaviour (#173). However, having considered this specific scenario involving a "never" expectation more carefully, I've decided to try to fix it with the changes in this PR.

Now a test like this will fail with an unexpected invocation error:

mock = mock('mock')
mock.stubs(:method)
mock.expects(:method).never
mock.method

unexpected invocation: #<Mock:mock>.method()
unsatisfied expectations:
- expected never, invoked once: #<Mock:mock>.method(any_parameters)
satisfied expectations:
- allowed any number of times, invoked never: #<Mock:mock>.method(any_parameters)

Closes #678. Also addresses #490, #131 & #44.

[davidstosik]

@floehopper Hi there! 👋🏻
I started seeing the warning introduced by #681 and would like to check what's expected to happen on a specific scenario.
Here it is:

require "test_helper"

class NeverTest < ActiveSupport::TestCase
  class Foo
    def self.create_if(condition)
      new if condition
    end
  end

  test "it creates only if condition is true" do
    Foo.expects(:new).never
    Foo.create_if(false)

    Foo.expects(:new).once
    Foo.create_if(true)
  end
end

This test currently passes, though it triggers the deprecation warning.

Are we expecting this to still work in the future?
If not, how can I write a test that follows the same logic as above?

[floehopper]

@davidstosik Thanks for reporting this. It does look as if something's not quite right with the deprecation warning logic. I'll try to look into it as soon as I can.

[floehopper]

@davidstosik I've attempted a fix for the deprecation warning logic in #686. If you have a chance, could you try running your tests against the fix-deprecation-warning-logic-for-expectation-with-never-cardinality branch of mocha to see whether (a) the deprecation warning goes away in the case you identified; and (b) there are no other unexpected regressions...? Oh, and just to be clear, I don't expect the test you described to fail when #679 is merged.