-
Notifications
You must be signed in to change notification settings - Fork 7
Add ignoreExtraArgs option
#36
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
Merged
+137
−32
Merged
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2029a3b
Add `ignoreExtraArgs` option
sirlancelot 8592cdb
Adjust `when` type to support `ignoreExtraArgs`
sirlancelot 5054078
Use variable name for conditional
sirlancelot a5ff0b7
Improve typing
sirlancelot 8ef6226
Pass any[] out if it was received
sirlancelot 36ae1ca
fixup: simplify types
mcous 0d660d7
fixup: fix arrity check, add test
mcous File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this arity check before the arguments - it makes me realize there's a subtle bug in the current implementation around explicit vs implicit
undefinedvaluesA few suggestions, in priority order
>in theignoreExtraArgscase?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
expectedArgumentslength is defined by the stub the user created with.calledWith(). This rehearsal is expected to be "correct", and soactualArgumentslength should exactly match in order for the stubbing to be satisfied, unlessignoreExtraArgsis true.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the following specification of
ignoreExtraArgs......with
ignoreExtraArgsset, what do you expect the following stubbing to do?My gut says
when(vi.fn(), { ignoreExtraArgs: true }).calledWith(undefined)should not matchspy(). However, with a!==check, this does match.Passing
undefinedexplicitly is extremely similar to, but not technically exactly the same as, omitting an argument, so I think if the user says "I expect an explicitundefined" it should be respected, even ifignoreExtraArgsis set