Wrap exceptions from the Assert.Equal comparer

[AndriySvyryd]

Test code

public class MyClass
{
    public int Id { get; set; }
    public string Name { get; set; }
}

public virtual bool AssertEqual(MyClass x, MyClass y)
{
    Assert.Multiple(
        () => Assert.Equal(x.Name, y.Name),
        () => Assert.Equal(x.Id, y.Id));

    return true;
}

[Fact]
public void Test()
{
    var x = new MyClass { Id = 1, Name = "One" };
    var y = new MyClass { Id = 2, Name = "Two" };
    Assert.Equal([x, y], [y, x], AssertEqual);
}

Expected

Assert.Equal() Failure: Collections differ
           ↓ (pos 0)
Expected: [MyClass { Id = 1, Name = "One" }, MyClass { Id = 2, Name = "Two" }]
Actual:   [MyClass { Id = 2, Name = "Two" }, MyClass { Id = 1, Name = "One" }]
           ↑ (pos 0)
Error:    Assert.Multiple() Failure: Multiple failures were encountered
          ----Assert.Equal() Failure: Strings differ
                     ↓ (pos 0)
          Expected: "One"
          Actual:   "Two"
                     ↑ (pos 0)
          ----Assert.Equal() Failure: Values differ
          Expected: 1
          Actual:   2

Actual

Assert.Multiple() Failure: Multiple failures were encountered
---- Assert.Equal() Failure: Strings differ
           ↓ (pos 0)
Expected: "One"
Actual:   "Two"
           ↑ (pos 0)
---- Assert.Equal() Failure: Values differ
Expected: 1
Actual:   2

Additionally, consider adding a overloads specifically for this type of assert, something like:

    public static void Equal<T>(
        T? expected,
        T? actual,
        Action<T, T> check)

    public static void Equal<T>(
        IEnumerable<T>? expected,
        IEnumerable<T>? actual,
        Action<T, T> check)

[bradwilson]

I'll have to think about this.

My gut reaction is that I think rather than "additionally", it would be that an Action<T, T> version did what you want, while the Func<T, T, bool> version continues to behave like it does today. The intent of the Func version is to return true or false, not to throw exceptions (from assertions or otherwise). I'm reluctant to change that behavior, and would be more comfortable with the Action version instead.

One thing that gives me a bit of pause is that the Action version would be unlike any other equality assertion behavior; everything else is about the true/false behavior.

[AndriySvyryd]

One thing that gives me a bit of pause is that the Action version would be unlike any other equality assertion behavior; everything else is about the true/false behavior.

The collection equality assertion would be similar in behavior to Assert.Collection, so perhaps it merits a new name - Assert.CollectionsEqual or similar. However, this would be hard to extend to the non-collection assertion. Though that one isn't as important to me, since it can be simulated by adding another check to Assert.Multiple.

[AndriySvyryd]

Another approach would be embedding Assert.Multiple in the call:

    public static void CompositeEqual<T>(
        T? expected,
        T? actual,
        params Action<T, T>[] checks);

    public static void CompositeEqual<T>(
        IEnumerable<T>? expected,
        IEnumerable<T>? actual,
        params Action<T, T>[] checks);

[bradwilson]

Hmm, this is an interesting suggestion. It has the value of being a "different" assert, and therefore the inherent confusion between Func<T, T, bool> vs. Action<T, T> goes away.

I'm not 100% sure the first version (non-enumerable) is necessary, but having both definitely offers some symmetry.

I'll poke around with this today and see how it feels.

[bradwilson]

After prototyping all the options and looking at the results, I think I'm going to go with a modification of your original suggestion.

I've updated Assert.Equal to record exceptions thrown during comparison, and report them via the normal inner exception mechanism. The resulting output I think gives you everything you need: the pointer into the collection where the exception occurred, and the exception information (including stack trace, which your example didn't show). I'm also updating Assert.NotEqual, though your original request did not ask for that (including mismatched item pointers, which have never been part of Assert.NotEqual before).

I've decided that this update is a debugging improvement: if you throw when we don't expect you to throw, we're providing additional information to help you understand why and how you threw. In your case, you were doing it with assertions, but that is a secondary reason to why I'm doing this. Even an inadvertent throw in the current system will only provide partial information (the exception), whereas with this update it will provide additional context (which element was being compared when the exception occurred).

This isn't exactly the feature I think you were asking for, because I'm not 100% convinced that that's a scenario that I necessarily want to support. Attempting to reconcile the differences between what "throw" means in Equal and NotEqual is what led me to the conclusion that this is just additional debugging assistance, and not a "special" way to think about writing a comparer. The comparers are there to give true/false information; throwing is an unexpected situation. Even so, I believe that the feature that I've implemented should still be usable in your scenario, even if it wasn't necessarily designed for it. 😄

Here's the output from your test with my changes:

Assert.Equal() Failure: Exception thrown during comparison
           ↓ (pos 0)
Expected: [MyClass { Id = 1, Name = "One" }, MyClass { Id = 2, Name = "Two" }]
Actual:   [MyClass { Id = 2, Name = "Two" }, MyClass { Id = 1, Name = "One" }]
           ↑ (pos 0)
---- Assert.Multiple() Failure: Multiple failures were encountered
-------- Assert.Equal() Failure: Strings differ
           ↓ (pos 0)
Expected: "One"
Actual:   "Two"
           ↑ (pos 0)
-------- Assert.Equal() Failure: Values differ
Expected: 1
Actual:   2
Stack Trace:
  Sample.cs(37,0): at CollectionAssertsTests.Test()
  ----- Inner Stack Trace -----
  Inner stack traces:
  ----- Inner Stack Trace #1 (Xunit.Sdk.EqualException) -----
  Sample.cs(26,0): at CollectionAssertsTests.<>c__DisplayClass1_0.<AssertEqual>b__0()
  ----- Inner Stack Trace #2 (Xunit.Sdk.EqualException) -----
  Sample.cs(27,0): at CollectionAssertsTests.<>c__DisplayClass1_0.<AssertEqual>b__1()

At this point in time, I am not planning any of the other suggested additions.

[bradwilson]

Available in v2 2.5.4-pre.5
Available in v3 0.1.1-pre.299

[AndriySvyryd]

This should be good enough ™
Thanks for the fast turnaround!