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
Alter the plugin to use stricter "to satisfy" semantics. #37
Conversation
The behaviour of allowing a set to compare against an array as part of "to satisfy" has shown itself to be surprising - exclude this behaviour from "to satisfy" and rename it as a separate assertion which still allows an array-like on the RHS. Perhaps more importantly a non-exhaustive satisfy passed with a subset on the RHS. This is another very surprising behaviour given arrays in core enforce an equal length. Switch to enforcing the Set being equal size by default.
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.
Ah, nice!
I thought I volunteered to do this, but luckily I didn't have a chance to work on it yet :)
Remember to fix the documentation tests too.
Oh I'm sorry if I took your thunder - just thought I'd provide a head start since I had a few spare mins the other day. |
No worries, I hadn't started :) |
Around the question of whether the members of a set should be allowed to have things missing - the documentation examples might be a good guide. The thing that gave me pause for thought was the following case:
The RHS specifies three items, but two of those things are satisfied by the same element on the LHS. I guess what this pokes at is what "exhaustively" really means when applied to Set comparisons. |
I still think that |
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.
LGTM 😌
@papandreou turns out there was already a (seemingly) undocumented |
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.
Looks good, just some objections against the to have items satisfying
implementation.
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.
LGTM ⚡
Co-authored-by: Andreas Lind <andreaslindpetersen@gmail.com>
Co-authored-by: Andreas Lind <andreaslindpetersen@gmail.com>
The behaviour of allowing a set to compare against an array as part
of "to satisfy" has shown itself to be surprising - exclude this
behaviour from "to satisfy" and rename it as a separate assertion
which still allows an array-like on the RHS.
Perhaps more importantly a non-exhaustive satisfy passed with a
subset on the RHS. This is another very surprising behaviour given
arrays in core enforce an equal length. Switch to enforcing the Set
being equal size by default.