tpierrain / NFluent

Smooth your .NET TDD experience with NFluent! NFluent is an ergonomic assertion library which aims to fluent your .NET TDD experience (based on simple Check.That() assertion statements). NFluent aims your tests to be fluent to write (with a super-duper-happy 'dot' auto-completion experience), fluent to read (i.e. as close as possible to plain English expression), but also fluent to troubleshoot, in a less-error-prone way comparing to the classical .NET test frameworks. NFluent is also directly inspired by the awesome Java FEST Fluent assertion/reflection library (http://fest.easytesting.org/)
Apache License 2.0
310 stars 53 forks source link

IsEquivalentTo wrongly fails when comparing dictonary-of-dictonaries, that are identical except the order of keys in child dicts #307

Closed tpehkone closed 4 years ago

tpehkone commented 5 years ago

Expected Behavior

When comparing structures with nested dictionaries, the order of keys of the nested dictionaries should not matter.

Current Behavior

IsEquivalentTo seems to allow any order of keys only if comparing trivial one level dictionaries. But for nested dictionaries the order matters, possibly making the comparison to fail.

The same happens, if trying to use: Contains() , Contains().Only() , IsEqualTo() . In fact, I was unable to find out any operation that would successfully compare nested dictionaries.

Furthermore, it looks like also lists-of-dictionaries suffer of the same issue, but did not yet test them thoroughly.

Possible Solution

Code that reproduce the issue

[Test]
public void Test_DictOfDict_IsEquivalentTo()
{
    var dictOf3_A = new Dictionary<string, string> { { "aa", "AA" }, { "bb", "BB" }, { "cc", "CC" } };
    var dictOf3_B = new Dictionary<string, string> { { "cc", "CC" }, { "aa", "AA" }, { "bb", "BB" } };
    var dictOf2 = new Dictionary<string, string> { { "cc", "CC" }, { "bb", "BB" } };

    var dictOf2Dict_A = new Dictionary<string, Dictionary<string, string>> { { "key1", dictOf2 }, { "key2", dictOf3_A } };
    var dictOf2Dict_B = new Dictionary<string, Dictionary<string, string>> { { "key1", dictOf2 }, { "key2", dictOf3_B } };

    Check.That(dictOf2Dict_A).IsEquivalentTo(dictOf2Dict_B);  // Fails -> Too picky on evaluating child-dict order
}

Context

Your Environment

dupdob commented 5 years ago

Thanks for raising this. Indeed IsEquivalentTo is not recursive as of now, i.e. does not perform any deep equivalence comparison. I need to decide if it requires adding a specific check (eg IsDeeplyEquivalent) or changing current behaviour. I tend to think the latter is the natural one, but I prefer not to rush behaviour decision.

In any case, expect a beta with a solution in the coming days.

tpehkone commented 5 years ago

Thanks for taking this up. In the end I am quite a bit after an NUnit-like IsEqualTo()" deep comparison algorithm, that can travel through any amount of nested lists and dictionaries, comparing them ordered way for list-like-structures, and unordered way for hashables, decided dynamically while traveling the data.

I.e. as an end-user, I could just call Check.That(complexStructA).IsEqualTo(complexStructB), and leave the heavy work & error reporting of actual data mismatch for the algorithm. (Here the "IsEqualTo" could be of course having any other name too).

Understanding that the current IsEqualTo does not quite do this by design, but I could raise an enhancement request for this kind of behavior, if it sounds sensible.

dupdob commented 5 years ago

Note that Lists are supported as you expect in the current version. I decided to ensure IsEquivalentTo to perform deep equivalence, against modifying IsEqualTo behaviour. Rationale: I think there is value in having strong equality (i.e order dependent) for IDictionary and I do not want to a specific check for that, since we already have IsEquivalent for weak equality.

dupdob commented 5 years ago

I will publish a beta version in a couple of hours. Note that as of now, deep equivalence is only supported for IDictionary implementation, which is only an issue when using custom IDictionary<K,V> implementations that do not also implement IDictionary. Not an issue for enumerations

dupdob commented 5 years ago

I published a V2.6.1 beta that fixes this issue. Can you please try it and confirm issue is solved? Package is available on MyGet: https://www.myget.org/feed/dupdobnightly/package/nuget/NFluent/2.6.1-beta-0204

tpehkone commented 5 years ago

Thanks for the prompt first fix on this issue. Dict-of-dict structure comparisons seem to work fine now, so maybe this issue can be considered closed.

As a heads-up however, I need to do some further testing to exactly pinpoint some remaining problematic ares, possibly raising up separate issues then - but that goes for the next week. Some initial finding with the beta:

Background: I have a rather extensive self-test set against an automatic-data-conversion-layer between 2 technologies, supporting conversion of for all kinds of array/list/dict nested constructs. NFluent comes into play now that we are trying to find a new, self-standing and clean assertion library to do all the heavy lifting of data comparison work.

dupdob commented 5 years ago

Thanks for providing me with all these details. I will gladly work with you regarding your use cases. I am looking for this kind of feedback to help me drive the evolution of the lib. Coming back to your comments:

  1. Contains... relies on strict equality, so this is not surprising. To be more specific, it relies on the default equality strategy, which can select using Check.EqualMode. So I can add a WeakEqual option (naming TBC) that would disregard the order
  2. IsEquivalent still has issues with dict of lists of dicts: I think it should not be the case; I will try to reproduce on my own if time permits but any sample you could provide would be helping of course.
  3. By params I assume you mean variable list of parameters, as in Check.That(dico).IsEquivalentTo(list1, list2, list3 ...). It may be linked to the previous issue.
  4. As discussed above, this could be controlled using the static parameter
  5. Usage of nested generics will probably get disappointing result, unless they implement IDictionary. The fact is nesting requires type erasure for recursion, the alternative being to declare all possible signatures, including multi-level nesting.

Also, if you are interested, we have a Gitter channel and a Slack for more interactive exchanges.

vivainio commented 5 years ago
dupdob commented 5 years ago

NFluent V 2.6.2 beta 205 is available on MyGet. It fixes the problem with IsEquivalent for List and other IEnumerables (except arrays).

@tpehkone: I would value your feedback on this version. My plan is to:

  1. implements what should be the proper behaviour for IsEquivalent for the major type
  2. extend to more exotic types (such as custom IDictionary<K,V> implementations)
  3. refactor to good design
  4. make a public release
dupdob commented 4 years ago

fixed in V 2.7.0