-
Notifications
You must be signed in to change notification settings - Fork 732
Excluding and Including options should fail when applied on types with value semantics #2571
Description
Description
I am updating the FluentAssertions package on a project from a very old version that would always do member-wise comparisons for Should().BeEquivalentTo(), even if the objects implemented IEquatable<T>. In general, I am ok with the new behavior, but here is a consequence I think is unexpected and undesirable: Explicit member exclusions are being completely ignored.
IMO, FluentAssertions should consider the fact that the implementation of IEquatable<T>.Equals is opaque and hence cannot be assumed to work in any particular way, so it should automatically fall back to perform member-wise comparisons when the assertion is configured with any type of explicit member exclusion.
In practice with the current behavior (which repros on 6.12 and 7.0.0 preview), I am forced to add ComparingByMembers<T>() specifying an explicit T (because it cannot be inferred by the compiler, which adds to the friction) for BeEquivalentTo() call that uses Excluding().
Reproduction Steps
// Paste this in a console app:
using FluentAssertions;
using FluentAssertions.Equivalency;
var actual = new SomeEquatableClass { Id = 1, Etag = "x" };
var expected = new SomeEquatableClass { Id = 1, Etag = "y" };
// This assertion fails, even if only Etag property has different values and it's being explicitly excluded:
actual.Should().BeEquivalentTo(expected, opt => opt.Excluding(o => o.Etag));
public class SomeEquatableClass : IEquatable<SomeEquatableClass>
{
public int Id { get; set; }
public string? Etag { get; set; }
public override bool Equals(object? obj)
{
return Equals(obj as SomeEquatableClass);
}
public bool Equals(SomeEquatableClass? other)
{
if (other == null)
return false;
return Id == other.Id && StringComparer.OrdinalIgnoreCase.Equals(Etag, other.Etag);
}
public override int GetHashCode()
{
return HashCode.Combine(Id, Etag);
}
}Expected behavior
Explicit exclusions should never be ignored, and this assertion should pass:
actual.Should().BeEquivalentTo(expected, opt => opt.Excluding(o => o.Etag));Actual behavior
Exclusions are ignored so this assertion fails:
actual.Should().BeEquivalentTo(expected, opt => opt.Excluding(o => o.Etag));Regression?
This is a regression but only from very old FluentAssertions versions like 3.5.0.
Known Workarounds
- Add a call to
ComparingByMembers<T>()wheneverExcluding()is also used. This is particularly painful becauseThas to be specified explicitly:actual.Should().BeEquivalentTo(expected, opt => opt.ComparingByMembers<SomeEquatableClass>().Excluding(o => o.Etag));
- Come up with your own exclusion extension that automates this and leverages type inference, like:
public static EquivalencyAssertionOptions<TExpectation> ExcludingByMembers<TExpectation>( this EquivalencyAssertionOptions<TExpectation> options, params string[] exclusions) { return options .ComparingByMembers<TExpectation>() .Excluding(memberInfo => exclusions.Any(e => memberInfo.Path.Equals(e, StringComparison.OrdinalIgnoreCase))); }
Configuration
Repros on FA 6.12 and 7.0.0 preview.
Used to work fine with FA 3.5.0.
Tested on both .NET Framework 4.x and .NET 8.
Other information
Apart from this, the way FluentAssertions now leverages IEquatable<T>.Equals forced us to debug a bunch of incorrect equality evaluation code in our domain classes. While this is ultimately a good thing and I am thankful to FA team for this 😄, it feels like a different concern from what the tests were initially designed to cover. I know that the behavior can be customized, so this is just a comment on the choice of default behavior.
Note: I am a newbie using FluentAssertions so I will be the first to admit that my mental model might be lacking. I apologize if I am being naive or if I missed any discussion in which it was decided that this was by design (I did a search on the repo, but I did not find anything that seemed relevant).
Are you willing to help with a pull-request?
Maybe 😄
I am not sure how long it can take me to understand the FluentAssertions codebase well enough.