freerange / mocha

A mocking and stubbing library for Ruby
https://mocha.jamesmead.org
Other
1.23k stars 158 forks source link

Error on `expects.with` with empty hash argument #657

Closed josesei closed 3 months ago

josesei commented 4 months ago

How to reproduce:

minitest (5.24.1) mocha (2.4.2) rails (6.1.7.8)

Ruby 3.3.0

require "test_helper"

class MyKlass
  def self.method_accepting_hash(h)
    "whatever"
  end
end

class MochaErrorTest < ActiveSupport::TestCase
  hash_with_value1 = {"key" => "value"}
  hash_with_value2 = {"key2" => "value2"}
  empty_hash = {}

  test "mocha success, hash with values" do
    MyKlass.expects(:method_accepting_hash).once.with(hash_with_value1).returns("whatever")
    MyKlass.expects(:method_accepting_hash).once.with(hash_with_value2).returns("whatever")
    MyKlass.method_accepting_hash(hash_with_value1)
    MyKlass.method_accepting_hash(hash_with_value2)
  end

  test "mocha success, empty hashes" do
    MyKlass.expects(:method_accepting_hash).once.with(empty_hash).returns("whatever")
    MyKlass.expects(:method_accepting_hash).once.with(empty_hash).returns("whatever")
    MyKlass.method_accepting_hash(empty_hash)
    MyKlass.method_accepting_hash(empty_hash)
  end

  test "mocha success first empty hash, then hash with values" do
    MyKlass.expects(:method_accepting_hash).once.with(empty_hash).returns("whatever")
    MyKlass.expects(:method_accepting_hash).once.with(hash_with_value1).returns("whatever")
    MyKlass.method_accepting_hash(empty_hash)
    MyKlass.method_accepting_hash(hash_with_value1)
  end

  test "mocha error first hash with values, then empty hash, weird error message?" do
    MyKlass.expects(:method_accepting_hash).once.with(hash_with_value1).returns("whatever")
    MyKlass.expects(:method_accepting_hash).once.with(empty_hash).returns("whatever")
    MyKlass.method_accepting_hash(hash_with_value1)
    MyKlass.method_accepting_hash(empty_hash)
  end
end

Failure:

Minitest::Assertion: unexpected invocation: MyKlass.method_accepting_hash({}) unsatisfied expectations:

test/support/mocha_error_test.rb:31:in `block in '

josesei commented 4 months ago

workaround is to use block matcher

josesei commented 4 months ago

@floehopper just in case

module ParameterMatchers
    # @private
    class PositionalOrKeywordHash < Base
      def matches?(available_parameters)
        parameter, is_last_parameter = extract_parameter(available_parameters)

        # return false unless HasEntries.new(@value).matches?([parameter]) <---- commenting this line makes the tests pass

Possibly you need that for something else, but I'd love to get that fixed

floehopper commented 4 months ago

@josesei Thanks for reporting this. I had independently come to the same conclusion as you as to where the problem lies. I've managed to write a couple of failing unit tests in #660 which reproduce the issue. That line of code is needed to handle nested matchers with keyword arguments (see test below), but clearly the implentation isn't quite right. My head is pretty scrambled right now, but I'll try to get it fixed as soon as I can.

https://github.com/freerange/mocha/blob/e62fa61e086199f7777d70e85371525f6c2afe03/test/unit/parameter_matchers/positional_or_keyword_hash_test.rb#L38-L41

josesei commented 4 months ago

sounds good, thx!

floehopper commented 4 months ago

@josesei ~Quick question - were these tests working with earlier versions of Mocha (e.g. v2.2.0)?~

Actually - ignore that!

floehopper commented 4 months ago

@josesei I've got some tidying up to do and some double-checking I haven't broken anything else, but I think the changes in the fix-regression-in-matching-hash-parameter branch should fix your problem. If you get the chance, I'd really appreciate it if you could see if it does fix the problem.

josesei commented 4 months ago

yep, it does work now! appreciate the effort!

it worked for me for ruby 2.6 and rails 5, mocha version mocha (1.13.0)

I guess it has to do with the keyword params compat thingy, but I'm sure you have it

floehopper commented 3 months ago

@josesei Thanks for testing. The problem was something I accidentally introduced in v2.3.0 (specifically https://github.com/freerange/mocha/commit/f94e250414335d822ec8d0a1331cb25cf90bf837) as a fix for #647 where a Hash parameter passed into Expectation#with was implicitly wrapped in a call to #has_entries. The latter does not require an exact match - only the specified entries have to match. The fix is to ensure that all entries match and that there are no extra entries. I hope to release the fix later today.

josesei commented 3 months ago

Makes sense, thanks for the insights and all your work!

floehopper commented 3 months ago

This fix has been released in v2.4.3.