-
Notifications
You must be signed in to change notification settings - Fork 214
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
Special case maps with only value keys #2257
Conversation
Closes #2256 When checking deep equality on Map instances (including maps nested in other collection types) we allow keys which are instances of other collection (and don't have a useful `operator ==`) and keys which are `Condition` callbacks that could potentially match multiple keys of the actual value. This is maximally flexible, but it loses the nested path information we are otherwise able to keep for iterables compared by index. Check whether expectation maps have any keys which need this special treatment, and if not, special case to an algorithm close to the `Iterable` algorithm which can enqueue nested checks and maintain the direct known path into the collection. Update the doc comment to describe the new behavior. This does potentially introduce confusion since a change which adds a `Condition` key has to potential to make _other_ keys in the same expectation behaved differently. The benefit is a more actionable failure output for the much more common case of checking expectations with hardcoded keys.
/// Deep equality checks for [Map] instances depend on the structure of the | ||
/// expected map. |
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.
@jakemac53 - WDYT about this behavior?
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.
Sounds pretty reasonable to me.
This looks great, thank you!
To make sure it's visible on this thread too: I just replied to a remark about this on the issue. It's possible to give a fun demo for this, in fact (#2256 (comment)), using as actual a SplayTreeMap with both 0.0 and -0.0 as keys, so that turning a different expected key from a plain value to a But I think it's the case that that sort of behavior can only come about when one of the maps involved doesn't behave consistent with how a Map is supposed to behave (like in that demo, where it has two |
I'm pretty happy with how this is working as I use it. I think we should land this. |
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. Package publish validation ✔️
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
Closes #2256
When checking deep equality on Map instances (including maps nested in
other collection types) we allow keys which are instances of other
collection (and don't have a useful
operator ==
) and keys which areCondition
callbacks that could potentially match multiple keys of theactual value. This is maximally flexible, but it loses the nested path
information we are otherwise able to keep for iterables compared by
index.
Check whether expectation maps have any keys which need this special
treatment, and if not, special case to an algorithm close to the
Iterable
algorithm which can enqueue nested checks and maintain thedirect known path into the collection.
Update the doc comment to describe the new behavior. This does
potentially introduce confusion since a change which adds a
Condition
key has to potential to make other keys in the same expectation
behaved differently. The benefit is a more actionable failure output for
the much more common case of checking expectations with hardcoded keys.