-
Notifications
You must be signed in to change notification settings - Fork 731
A collection of similarly typed key-value pairs should be equivalent to a dictionary #1603
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
A collection of similarly typed key-value pairs should be equivalent to a dictionary #1603
Conversation
698510a to
8e247d4
Compare
1d1efe4 to
6aeb59e
Compare
0fcad04 to
801c7fc
Compare
801c7fc to
3dd80f7
Compare
jnyrup
left a comment
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.
Can you give a high level description of what's changed.
I have a bit of a hard time separating the restructuring from the change in logic.
Have you tried my suggestion from
#1598 (comment) ?
Tests/FluentAssertions.Specs/Equivalency/DictionaryEquivalencySpecs.cs
Outdated
Show resolved
Hide resolved
| // Assert | ||
| act.Should().Throw<XunitException>() | ||
| .WithMessage("*expectation*keys*Int32*compatible types*IDictionary`2[System.String,System.String]*"); | ||
| .WithMessage("Expected the expectation (of type Dictionary<int, string>) to be a dictionary or collection of key-value pairs that is keyed to type System.Int32*"); |
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.
expectation and actual seems flipped in the act.
| bool isDictionary = DictionaryInterfaceInfo.TryGetFrom(expectationType, "expectation", out var expectedDictionary); | ||
| if (comparands.Expectation != null && isDictionary) |
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.
Consider only calling DictionaryInterfaceInfo.TryGetFrom if comparands.Expectation != null
| return false; | ||
| } | ||
|
|
||
| private static bool AssertIsCompatiblyTypedDictionary(Type expectedType, object subject) |
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.
There's a now outdated comment on line 118 mentioning AssertIsCompatiblyTypedDictionary
| if (expectedDictionary.TryConvertFrom(comparands.Subject, out var convertedSubject)) | ||
| { | ||
| comparands.Subject = convertedSubject; | ||
| DictionaryInterfaceInfo.TryGetFrom(comparands.Subject.GetType(), "subject", out actualDictionary); |
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.
double check: is the return value of TryGetFrom unimportant?
Sorry. Forgot about that one. Will do.
Not really. The old implementation was kind of an accidental side-effect. Since the expectation is a dictionary, I decided to implement this where it belongs. Now it's completely symmetric.
I understand, but I don't think I could have avoided that. The idea of extracting some of the logic happened while I was trying to implement the algorithm. |
36fcacb to
6c9c2c8
Compare
6c9c2c8 to
51a4bd0
Compare
… to a dictionary
51a4bd0 to
7cf777d
Compare
Because of some accidental design choices, in previous releases, when the expectation is a dictionary, it was picked up by the
GenericEnumerableEquivalencyStep. This resulted in anIDictionary<TKey, TValue>being treated as a collection ofKeyValuePair<TKey, TValue>. In 6.0, some of the semantics of the equivalency steps changed in such a way that a dictionary is now picked up by theGenericEquivalencyStep. This is expected, but had the side-effect that when a dictionary is expected, it really expects a dictionary.This PR changes
GenericEnumerableEquivalencyStepso it tries to convert a collection of key-value pairs back to a dictionary and then does the equivalency check.Fixes #1598