Regression in `.expects(...).with(...)` around keyword arguments between 2.4.2 and 2.4.3 (not fixed)

[davidstosik]

In short

A test that used to work with Mocha 2.3.0, 2.4.0 and 2.4.2 stopped working in Mocha 2.4.3 and still does not work in the latest release (2.4.5). At such, this feels either like a regression, or a "breaking change" that should have warranted a major version bump.

What's the problem?

This involves expecting a method call with partial keyword arguments. For example, foo.expects(:bar).with(key: "value").

It used to be that calling the method with more keyword arguments (eg. foo.bar(key: "value", other_key: "anything")) would match the expectation, but it does not anymore.

This is very likely the PR that broke this scenario: https://github.com/freerange/mocha/pull/660, since it is the only change between 2.4.2 and 2.4.3.

Since that PR also introduces a test that makes it clear that new behavior is expected, I imagine this is not a regression, but actually a fix. Since it changes a behavior that I verified from 2.3.0, I'd say this should have required a bit more warning.

Reproduction steps

Here's a script to reproduce it:

# test.rb
require "rubygems"

gem "minitest"
gem "mocha", ENV.fetch("MOCHA_VERSION", nil)

require "minitest/autorun"
require "mocha/minitest"

class MochaTest < Minitest::Test
  def test_mocha
    mock = mock()
    mock.expects(:method).with(key: "value")
    mock.method(key: "value", key2: "value")
  end
end

require "mocha/version"
puts "="*30, "Mocha version: #{Mocha::VERSION}"

$ for v in 2.3.0 2.4.0 2.4.2 2.4.3 2.4.5; do gem install "mocha" --version $v; done
$ for v in 2.3.0 2.4.0 2.4.2 2.4.3 2.4.5; do MOCHA_VERSION=$v ruby test.rb; done
==============================
Mocha version: 2.3.0
 # (success)
==============================
Mocha version: 2.4.0
 # (success)
==============================
Mocha version: 2.4.2
 # (success)
==============================
Mocha version: 2.4.3
  1) Failure:
MochaTest#test_mocha [test.rb:19]:
unexpected invocation: #<Foo:0x668>.bar("arg1", key: "value", key2: "value")
unsatisfied expectations:
- expected exactly once, invoked never: #<Foo:0x668>.bar("arg1", key: "value")
==============================
Mocha version: 2.4.5
 # (same failure)

[davidstosik]

For completeness, I checked 2.2.0 as well and realized that the behaviour "introduced" by 2.4.3 was actually restored from 2.2.0, so "regression fix" I guess..?

Still think that after this many releases, there should have been a clearer warning.

[floehopper]

@davidstosik

I can only apologise that you were caught out by this. We have a very comprehensive set of both unit and acceptance tests, but inevitably sometimes accidental changes in behaviour do slip through the net, because of some missing test coverage. In this case it was unfortunate that there was a period of a month or two between the release which included the regression and when the regression was identified and fully fixed, which is presumably why this caused a problem for you.

I'm not sure we could have handled the issue much better. However, taking your feedback on board, I'm going to review the release notes for the relevant versions and see if we can augment them retrospectively with some clearer warnings about the regressions and/or add something to the known issues in the README. See https://github.com/freerange/mocha/issues/676.

On that basis, I'm going to close this issue, but feel free to re-open if you think I've missed something.

[davidstosik]

Thank you for your reply! I understand what happened, and I agree it would be nice to increase visibility. 👍🏻 Just one nitpick though:

a period of a month or two

• v2.3.0 was tagged on May 18 (bug introduced)
• v2.4.2 was tagged on July 18 (bug fixed)

Damn, my bad, that's exactly 2 months! Since we attempted to upgrade from v2.2.0 just now, I was expecting that bug to have lived for almost 6 months, but I was mistaken. 👌🏻

[davidstosik]

@floehopper After a whole day slogging through broken tests, I'd like to suggest something. (I can't reopen myself.)

Would it be possible to introduce a new version of the gem that restores the old (broken) behaviour, either behind a config flag and/or raising a deprecation?

Here, the change could look something like this:

        partial_match_behaviour = Mocha.configuration.positional_or_keyword_hash_partial_match_behaviour
        unless HasEntries.new(@value, exact: true).matches?([parameter])
          return if partial_match_behaviour == :fail || !HasEntries.new(@value, exact: false).matches?([parameter])

          raise SomeDeprecation if partial_match_behaviour == :raise
        end

[floehopper]

@davidstosik

I've now updated the release notes to include warnings about the various regressions against each relevant version.

Would it be possible to introduce a new version of the gem that restores the old (broken) behaviour, either behind a config flag and/or raising a deprecation?

Many thanks for this suggestion. I've decided not to implement it for now, but if I get any more similar reports, I'll definitely consider doing so.