Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent return value of stubs() / expects() #524

Closed
vlad-pisanov opened this issue Apr 18, 2022 · 7 comments · Fixed by #525
Closed

Inconsistent return value of stubs() / expects() #524

vlad-pisanov opened this issue Apr 18, 2022 · 7 comments · Fixed by #525
Assignees

Comments

@vlad-pisanov
Copy link

According to the docs, the result of stubs() and expects() is the "last-built expectation which can be further modified by methods on Expectation. This applies to both .stubs(:foo) and .stubs(foo: 1) invocations.

This seems to be true when stubbing methods on any class except Mock objects. For Mock objects, the return value is sometimes an Expectation, and sometimes a Hash.

# key-value stubbing returns the right thing on String ✔️
String.stubs(foo: 1, bar: 2)
#<Expectation:0x7f9e73aa6fe0 allowed any number of times, invoked never: String.bar(any_parameters) >

# key-value stubbing returns the wrong thing on Mock ❌
obj = stub()
obj.stubs(foo: 1, bar: 2)
# {:foo=>1, :bar=>2}

# key-stubbing returns the right thing on Mock ✔️
obj.stubs(:foo)
#<Expectation:0x7f9e73aa6fe0 allowed any number of times, invoked never: #<Mock:service>.foo(any_parameters) >
@floehopper
Copy link
Member

@vlad-pisanov Thanks for opening this issue. You are correct that the behaviour does not match the documentation and that seems to have been the case since the feature was first introduced 13 years ago!! I think fixing it should be relatively straightforward, but I am slightly questioning whether the documented behaviour is actually sensible. Since I don't think the order of entries in a Hash is defined, it may not be obvious which is the "last" expectation that gets returned. I wonder if it might be less surprising to return an array of expectations...? Do you have any thoughts? Do you have a specific use case that would help guide me?

@floehopper floehopper self-assigned this Apr 18, 2022
@vlad-pisanov
Copy link
Author

@floehopper A list of expectations might be nice, but arguably overkill. I like how the docs say that:

object = mock()
object.stubs(:stubbed_method_one => :result_one, :stubbed_method_two => :result_two)

# is exactly equivalent to

object = mock()
object.stubs(:stubbed_method_one).returns(:result_one)
object.stubs(:stubbed_method_two).returns(:result_two)

If it's exactly equivalent, then the result would be the last Expectation defined. As of Ruby 1.9.1, Hash keys enumerate in the order they were inserted so it should be predictable.

My use case is actually simple, in our codebase, we use this shortcut quite a bit:

UserLogic.expects(age: 42).twice

^ if this syntax worked even with a single key on Mock objects, we'd be all set. 😄

@floehopper
Copy link
Member

@vlad-pisanov

Thanks - it's helpful to have that context! What you say makes a lot of sense to me.

As of Ruby 1.9.1, Hash keys enumerate in the order they were inserted so it should be predictable.

I thought hash keys being ordered according to insertion order wasn't part of the Ruby "spec" and thus not guaranteed for all flavours of Ruby, but I might be out-of-date on that understanding. The most authoritative information I've been able to find after a quick internet search is this Stack Overflow answer, but that's very old and inconclusive. However, I note that the following spec is in the current Ruby repo, so maybe it's OK to rely on after all:

https://github.com/ruby/spec/blob/aaf998fb8c92c4e63ad423a2e7ca6e6921818c6e/core/hash/keys_spec.rb#L6-L14

floehopper added a commit that referenced this issue Apr 20, 2022
Even when passed a hash of method names vs return values. This is the
same behaviour as for ObjectMethods#expects & #stubs and the behaviour
described by the documentation for Mock#expects & #stubs.

Fixes #524.
floehopper added a commit that referenced this issue Apr 20, 2022
Even when passed a hash of method names vs return values. This is the
same behaviour as for ObjectMethods#expects & #stubs and the behaviour
described by the documentation for Mock#expects & #stubs.

Fixes #524.
@floehopper
Copy link
Member

@vlad-pisanov I've just merged a fix (#525). Is there any chance you could try out the latest version in main, i.e. by using something like gem 'mocha', github: 'freerange/mocha' in your Gemfile, to see if it resolves your issue. If all is well I'll try to get a release out.

@vlad-pisanov
Copy link
Author

@floehopper works like a charm! Thank you 🙏

@floehopper
Copy link
Member

@vlad-pisanov Thanks for testing it. We'll try to get a release out as soon as we can.

floehopper added a commit that referenced this issue Apr 25, 2022
I've made this a minor version bump, because technically it does change
some published behaviour very slightly (#524) albeit to bring it *into*
line with the existing documentation!
@floehopper
Copy link
Member

@vlad-pisanov This was released in v1.14.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants