A collection of similarly typed key-value pairs should be equivalent to a dictionary#1603
Conversation
dennisdoomen
force-pushed
the
Fix/CollectionOfKeyValuePair
branch
7 times, most recently
from
698510a to
8e247d4
Compare
dennisdoomen
force-pushed
the
Fix/CollectionOfKeyValuePair
branch
8 times, most recently
from
1d1efe4 to
6aeb59e
Compare
dennisdoomen
force-pushed
the
Fix/CollectionOfKeyValuePair
branch
2 times, most recently
from
0fcad04 to
801c7fc
Compare
dennisdoomen
changed the title
dennisdoomen
force-pushed
the
Fix/CollectionOfKeyValuePair
branch
from
801c7fc to
3dd80f7
Compare
jnyrup
left a comment
jnyrup
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
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. |
dennisdoomen
force-pushed
the
Fix/CollectionOfKeyValuePair
branch
2 times, most recently
from
36fcacb to
6c9c2c8
Compare
dennisdoomen
force-pushed
the
Fix/CollectionOfKeyValuePair
branch
from
6c9c2c8 to
51a4bd0
Compare
… to a dictionary
dennisdoomen
force-pushed
the
Fix/CollectionOfKeyValuePair
branch
from
51a4bd0 to
7cf777d
Compare
2 participants
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