freerange / mocha

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

Expectations with extra arguments are now "unexpected invocations" #667

Closed cpjmcquillan closed 3 months ago

cpjmcquillan commented 3 months ago

First of all, thanks for maintaining mocha! 💚

I'm not sure if this is a regression, or expected behaviour now.

Behaviour changed in v2.4.3.

Before, I could do something like this:

mock = mock()
mock
  .expects(:method)
  .with(
    "test",
    hello: "world",
    goodbye: "world"
  )
mock.method(
  "test",
  hello: "world",
  goodbye: "world",
  hello_again: "world"
)

and the test would pass.

Now, it fails.

If this is a regression I'm happy to fix it, but I don't want to do any unnecessary work!

Let me know if you need any more information.

floehopper commented 3 months ago

@cpjmcquillan

Thanks for reporting this.

Before, I could do something like this:

Do you know what version of Mocha you were using in the "before" times?

I thought that https://github.com/freerange/mocha/commit/5e6a07b2710dac76e9346def561ca0d44765bf86 was itself a fix for a regression introduced in https://github.com/freerange/mocha/commit/f94e250414335d822ec8d0a1331cb25cf90bf837 which was released in v2.3.0. So did your test pass in v2.2.0 or earlier?

cpjmcquillan commented 3 months ago

Do you know what version of Mocha you were using in the "before" times?

It passes in v2.4.2

floehopper commented 3 months ago

It passes in v2.4.2

Yes, but my suspicion is that v2.4.2 was a release that contained the original regression which was introduced in v2.3.0 (released on 17 May 2024) and fixed in v2.4.3 (released on 22 Jul 2024). The key question is does your test pass in v2.2.0 before the original regression was introduced - my suspicion is that it won't. When I have a bit of time I will check this myself, but let me know if you work it out before me.

If my understanding is correct, then I won't be regarding the change described in this issue as a regression. I realise this is all very confusing and I regret that the original problem wasn't spotted sooner.

floehopper commented 3 months ago

If my understanding is correct, then you should be able to obtain the behaviour you want by using ParameterMatchers#has_entries as follows:

mock = mock()
mock.expects(:method).with("test", has_entries(hello: "world", goodbye: "world"))
mock.method("test", hello: "world", goodbye: "world", hello_again: "world")
floehopper commented 3 months ago

@cpjmcquillan I've now confirmed that the test you added in https://github.com/cpjmcquillan/mocha/commit/2f62bd9b1a6b01b89da70f491f1d290c7667ab3a behaves as follows:

This is as I expected, i.e. your test relies on the regression accidentally introduced in v2.3.0 and fixed in v2.4.3. Thus the issue you have reported is not a bug and I won't be trying to fix it. I hope that makes sense.

cpjmcquillan commented 3 months ago

Was just about to reply with the same. Thank for the context and help 👌