-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
Improve docs for Expectation#with
when it takes a block argument
#682
Labels
Comments
floehopper
added a commit
that referenced
this issue
Nov 13, 2024
In the previous commit, the changes meant that a block provided to `Expectation#with` was being invoked three times instead of once. The original behaviour was not explicitly documented and there is code in the wild [1] that relies on `Expectation#with` only being called once per invocation. I'm not convinced that test code should be relying on this behaviour, but from a performance point-of-view, it probably makes sense to avoid calling the matching methods on `ExpectationList` multiple times unnecessarily. This commit changes the code in `Mock#handle_method_call` to avoid unnecessary calls to these methods. I've created #682 to suggest improving the docs for `Expectation#with` when it takes a block argument to address the ambiguity that has become apparent. [1]: #681 (comment)
floehopper
added a commit
that referenced
this issue
Nov 13, 2024
In the previous commit, the changes meant that a block provided to `Expectation#with` was being invoked three times instead of once. The original behaviour was not explicitly documented and there is code in the wild [1] that relies on `Expectation#with` only being called once per invocation. I'm not convinced that test code should be relying on this behaviour, but from a performance point-of-view, it probably makes sense to avoid calling the matching methods on `ExpectationList` multiple times unnecessarily. This commit changes the code in `Mock#handle_method_call` to avoid unnecessary calls to these methods. I've created #682 to suggest improving the docs for `Expectation#with` when it takes a block argument to address the ambiguity that has become apparent. [1]: #681 (comment)
floehopper
added a commit
that referenced
this issue
Nov 24, 2024
In the previous commit, the changes meant that a block provided to `Expectation#with` was being invoked three times instead of once. The original behaviour was not explicitly documented and there is code in the wild [1] that relies on `Expectation#with` only being called once per invocation. I'm not convinced that test code should be relying on this behaviour, but from a performance point-of-view, it probably makes sense to avoid calling the matching methods on `ExpectationList` multiple times unnecessarily. This commit changes the code in `Mock#handle_method_call` to avoid unnecessary calls to these methods. I've created #682 to suggest improving the docs for `Expectation#with` when it takes a block argument to address the ambiguity that has become apparent. [1]: #681 (comment)
floehopper
added a commit
that referenced
this issue
Dec 8, 2024
Although there was already an example for this and the `yield`, `yieldparam` & `yieldreturn` YARD tags were specified, I think it's helpful to add some explanatory text to the method description. Closes #682.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The current docs don't mention the block argument in the method description, although there is an example of its use and it does appear in the list of arguments.
It might also be worth saying there are no guarantees that the matcher won't be called more than once per invocation...?
The text was updated successfully, but these errors were encountered: