Add support for options hashes without braces #17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
searls
force-pushed
the
calebhearth-arity-kwargs-opts-hash
branch
from
2ebe94e to
32eef57
Compare
jasonkarns
reviewed
Nov 17, 2022
While Ruby supports options hashes without braces, Mocktail currently does not.
Co-authored-by: Justin Searls <searls@gmail.com>
Co-authored-by: Caleb Hearth <caleb@calebhearth.com>
This is both to facilitate using the ignore_extra_args option as well as to allow our Mocktail::SimulatesArgumentError to intercept any actual arg errors of mocked methods
…y (wastefully) instantiating HandlesDryCall each time a method is dispatched reworking this particular thing to be static or memoized might be worth it. do benchmarks first
…cit block parameters There are two things happening here This was tricky and required thinking about the implications of the `...` operator. Right now, adding `...` in a method signature is a shorthand way of saying "i don't care about any additional args, ANY supplied kwargs, and ANY supplied block" as a way to forward args more easily than defining all *args, **kwargs, &blk in a signature and then referencing them again in a call. 1. Because any method can receive and call a block even without converting it to a proc in the method signature (with `yield`), we can never know if a faked method might be called with a block and b/c you can stub and verify blocks conditionally, it basically means we MUST tack `&blk` at the end of every single signature we generate. This means we can't produce signatures with `...` because `..., &blk` is not valid 2. The way that Mocktail works is by deconstructing WHATEVER the user sends and passing it to `HandlesDryCall#handle(dry_call)` which requires to know all the extra args/kwargs/block in the event that the original method signature contains "...". Because we care about those values we need to name them, so I defaulted them to `args`, `kwargs`, and `blk` since that's what we use in most of the Mocktail codebase
…owable, we can stop mixing levels of abstraction here and reference the binding's local variable at method dispatch-time
…exposes us to all kinds of closure-scope argument name shadowing risk
…tail and using `__send__(:binding)` to slurp up the real binding if it's shadowed. Not even 9am yet and I feel like I need a second shower.
…eed to write a test for
…of concatenating the two subsets
calebhearth
force-pushed
the
calebhearth-arity-kwargs-opts-hash
branch
from
517b969 to
1c12118
Compare
calebhearth
approved these changes
Nov 17, 2022
calebhearth
force-pushed
the
calebhearth-arity-kwargs-opts-hash
branch
2 times, most recently
from
16305ac to
1c12118
Compare
Co-authored-by: Caleb Hearth <caleb@calebhearth.com>
This reverts commit 06029da. This broke tests on 3.0 in CI
|
Feelsgoodman |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Picks up the work of #14 (PR sent b/c I didn't have push access to @calebhearth's fork)