Include differences in output for Is.EquivalentTo

[appel1]

It'd be useful if the output for Is.EquivalentTo included at least one of the differences. Similar to how Is.EqualTo outputs the first index with a difference.

As it is right now you have to debug the test to figure out why the test failed.

        [Test]
        public void DummyTest()
        {
            var expected = Enumerable.Range('a', 12).Select(n => (char)n);
            var actual = expected.Take(11);

            Assert.That(actual, Is.EquivalentTo(expected));
        }

Result StackTrace:  at Dummy.DummyTest() in Dummy.cs:line 20
Result Message: 
Expected: equivalent to < 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'... >
  But was:  < 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'... >

[CharliePoole]

Since Equivalent collections may not be in the same order, it's not that simple to come up with an index where the assertion failed. It is possible to list some of the non-matching items, either as "extras", "missing" or both.

[JustinRChou]

I wonder if using Linq would be good enough to use for this?

var same = expected.Intersect(actual);
// a,b,c,d,e,f,g,h,i,j,k
var difference = actual.Except(expected).Union(expected.Except(actual));
// l

[CharliePoole]

It's a way to implement the constraint and to get differences... but I think this is more about what we want the error message to look like. It would be interesting to re-implement the constraint using Linq though.

[jnm2]

If you use a hash set and a loop, you can perform both set operations in one pass whereas LINQ will necessarily duplicate some of of the work.

[jnm2]

I really like the breakdown of Actual, Expected, Missing, and Unexpected.

[CharliePoole]

So, in this example, something like...

Expected: equivalent to < 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'... >
  But was:  < 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'... >
 Missing: <'l'>

The EqualConstraint already does this, using "Extra" and "Missing". It's a fairly easy implementation although not giving an "index" as requested. Nevertheless, could be worth doing as an improvement.

[jnm2]

Extra instead of Unexpected then, a bit more symmetrical and terser that way.

Does IsEquivalent make sure that each collection has the same number of each value? E.g. is new[] { 3 } equivalent to new[] { 3, 3 }? In that case you'd use a Dictionary<, int> instead of a HashSet<> but the loop would be almost identical.

[CharliePoole]

Well, I could say "Write a test" (kind of like telling your kids "Look it up!") but... yes, it does. 😄 The current implementation uses a CollectionTally object with an internal List. This could be pretty time-consuming for large sets but I haven't seen any issues with it over the years.

I don't think you can replace CollectionTally with pure Linq expressions because it has to deal with various user modifiers like tolerance, ignoring case and (the worst) treating arrays as arrays versus collections - i.e. does the shape of the array matter. This is all pretty complex but is handled by NUnitEqualityComparer, which hides most of the dirty bits.

CollectionTally could be modified to leave behind the lists of "extra" and "missing" elements pretty easily. It could conceivablly use a dictionary internally, provided the dictionary made use of NUnit's definition of equality. My own inclination would be to change only what is needed to get out a better report.

[jnm2]

Awesome, so in that case I imagine you'd want to keep around the list of equal items and report each one in case the string representation differs even though the equality comparer considers them the same?

[CharliePoole]

Currently, we don't report each one but limit it to (I think) 10. I wouldn't want to see an error message that listed thousands of items, for example.

As far as items reported unequal but with the same string representation, I think we can better catch them and fix them when they come up in isolation, rather than deep inside an EquivalentTo test. We have always treated such things as bugs and currently have a few such items reported as issues.

Anyway, when we report things as actual, expected, missing or extra, we can only report using a string representation. 😄 Other than using the debugger, there's no other way to see them.

[jnm2]

So if I do:

Assert.That(
    new[]
    {
        new User(id: 1, name: "Username changed"), 
        new User(id: 1, name: "Username changed again")
    },
    Is.EquivalentTo(new[]
    {
        new User(id: 1, name: "Username")
    });

_{(Edit: swapped expected and actual)}

And User.Equals only compares id and User.ToString returns the name, what would you expect to see reported?

[JustinRChou]

I would imagine that it produces something akin to
Missing : <Username changed again> or Extra : <Username changed again>
If equivalent would work like CollectionAssert.AreEqual(actual, expected).

And I see that there is commented code in 'DisplayEnumerableDifferences' in EqualConstraintResult for the Extra and Missing data? But it seems like Is.Equivalent returns a ConstraintResult.

[CharliePoole]

@JustinRChou For the message I'd prefer to see both Missing and Extra. In the case of equality, that's not possible, but it is for equivalence.

Is.Equivalent returns a CollectionEquivalentConstraint. That constraint does not override ApplyTo and so simply returns a ConstraintResult. Constraints that want more control over the display of errors need to provide a derived ConstraintResult type. For example, that's what EqualConstraint does with EqualConstraintResult.

EqualConstraintResult displays missing and extra elements for collections but not IEnumerables. The latter could be handled but would take a bit more effort. It would also not work for non-repeatable enumerations, although that's probably not a big concern.

[jnm2]

It would also not work for non-repeatable enumerations, although that's probably not a big concern.

In the past I've used an IEnumerable wrapper that takes an IEnumerator and caches items as you go so that you only enumerate once even though the wrapper can be enumerated multiple times. It might be handy to have around NUnit.

[OmicronPersei]

Looking to pick this up.

It seems that the bulk of the work is to implement a customized ConstraintResult type. Then we'll need to override CollectionEquivalentConstraint.ApplyTo<TActual>(TActual actual) to instead return this new ConstraintResult type. Should be able to expand the existing CollectionTally class to hold onto both missing and extra values. The newly created ConstraintResult would make use of the results CollectionTally came up with to serialize some output with a maximum amount of displayed "differing" elements.

[CharliePoole]

Sounds reasonable.

[jnm2]

@OmicronPersei It's yours.

[rprouse]

@OmicronPersei I've sent you an invitation to the Contributors team. That will allow us to assign issues to you. Once you accept the invitation, one of us can assign this issue. Welcome.

[OmicronPersei]

@rprouse Thank you! :)