fluentassertions icon indicating copy to clipboard operation
fluentassertions copied to clipboard

Add an option to BeEquivalentTo that compares types of objects recursively.

Open AxCoder opened this issue 8 years ago • 23 comments

I am trying to compare AST and found that BeEquivalentTo has no option to instruct the library to check types on comparing. For example, I expect the way to modify the following test code to fail because of class names do not match. This behavior should be recursive (so if property value does not have exactly the same type, comparison should fail). I see that stackoverflow has a question on this.

class OneThing
{
	public string Name => "Test";
}
class OtherThing
{
	public string Name => "Test";
}

[TestMethod]
public void MyTestMethod()
{
	new OneThing().Should().BeEquivalentTo(new OtherThing(), o => o.RespectingRuntimeTypes());
}

AxCoder avatar Mar 10 '18 12:03 AxCoder

If your AST class overrides bool Equals(object obj) a workaround right now would be to use oneAST.Should().Be(anotherAST); should work.

But I agree that it could be beneficial to have some option on BeEquivalentTo that made it more strict.

Just for the note: The purpose of RespectingRuntimeTypes is compare the members of the runtime type of the expectation. The default is to compare the members of the compile time type of the expectation.

class Base
{
    public string Name { get; set; }
}

class Derived : Base
{
    public int Age { get; set; }
}

[Fact]
public void MyTestMethod()
{
    Base a = new Base();
    Base b = new Derived() { Age = 42 };

    a.Should().BeEquivalentTo(b); // Success
    a.Should().BeEquivalentTo(b, o => o.RespectingRuntimeTypes()); // Fail
}

jnyrup avatar Mar 10 '18 15:03 jnyrup

Thank you.

  • overriding Equals takes some time
  • tried to add a property ClassName, but is stopped working when I inherited from collection:
class Base: List<string>
{
	public string ClassName => GetType().Name;
}

class OneThing: Base
{
	public string Name => "Test";
}

class OtherThing: Base
{
	public string Name => "Test";
}

[TestMethod]
public void MyTestMethod()
{
	new OneThing().Should().BeEquivalentTo(new OtherThing(), o => o.RespectingRuntimeTypes());
	//new OneThing().ClassName.Should().Be(new OtherThing().ClassName);
}

AxCoder avatar Mar 10 '18 16:03 AxCoder

The framework currently assumes that when a class implements IEnumerable<> or IEnumerable, two instances of that class are equivalent, when the implemented enumerable parts are equivalent.

If possible you could change Base to use Composition over Inheritance.

jnyrup avatar Mar 10 '18 17:03 jnyrup

Thank you. Introducing ClassName property and extracting collection to a separate property fixed my problem. But generally I think, that adding options for comparing classes and compare both properties and elements would help to reduce false passing tests (they are more difficult to detect then false failing tests and can lead to hiding bugs). Maybe even introduce a new method with different defaults (respect runtime types, compare class names, compare both properties and contents)?

AxCoder avatar Mar 12 '18 11:03 AxCoder

Is there any option for equivalence to make sure the element types (not their properties, the elements themselves) are not the same?

As an example, I have two collections with different element types, although the elements in each collection have the exact same properties and property types

_result.Should().BeEquivalentTo(_events, opt =>
                    opt.RespectingRuntimeTypes());

I would expect that assertion to fail as the element types are different, but it passes.

iberodev avatar Sep 06 '19 14:09 iberodev

What's the type of _events?

dennisdoomen avatar Sep 06 '19 15:09 dennisdoomen

it's a list of different implementations of the same marker interface IPersistableEvent.

It's for a sample like the following where there are two collections. One of them stores some objects with certain properties. The other stores some different objects of a different class although this class has the same name and the same properties

//...
_events =
     new List<IPersistableEvent>
     {
         new FakeEventOne(),
         new FakeEventTwo()
     };
_result = 
   new List<object>{
      new AnotherNamespace.FakeEventOne(),
      new AnotherNamespace.FakeEventTwo()
   };

and basically I was thinking that asserting that both are equivalent respecting runtime types would fail. But it seems it's only taking into consideration property types and not the element types themselves.

iberodev avatar Sep 06 '19 15:09 iberodev

I think this is related?

This used to work:

result.ShouldBeEquivalentTo(new IFragmentInjector[]
{
	new BibliographicIdentifierInjector(),
	new AgeClassificationInjector(),
	new SalesRightsCountryExpanderInjector(mock.Object)
});

Upgraded to the latest version and this now fails with "No members were found for comparison". There are no properties on the interface so I guess I can understand why. However, all I need to check is that the types are the same - not sure how I can do this now.

grahambunce avatar Sep 24 '19 09:09 grahambunce

@grahambunce We had a lot of changes between 4.X and 5.0, see https://www.continuousimprover.com/2018/02/fluent-assertions-50-best-unit-test.html#redefining-equivalency You should probably use

result.Should().BeEquivalentTo(
  new IFragmentInjector[]{...}, 
  opt => opt.RespectingRuntimeTypes())

If that does not help, please open a new issue.

jnyrup avatar Sep 24 '19 10:09 jnyrup

@jnyrup Ok - tried that and no, it doesn't work.

grahambunce avatar Sep 24 '19 10:09 grahambunce

The entire existence of the BeEquivalentTo API is to allow you to compare the structure of an object graph for equivalency, not equality. So it's by design that it will only look at property and field values and never at the exact type (unless that type has value semantics).

dennisdoomen avatar Sep 24 '19 18:09 dennisdoomen

@dennisdoomen ok... I guess that makes sense. It’s just the old api passes and it’s been removed so the alternative api fails because the object being compared is an interface that has no types. Perhaps the old api had a bug?

Is there a way to provide a custom comparer? I see something but it looks complicated with a lot of things to implement when a func that returns true/false would suffice

grahambunce avatar Sep 25 '19 18:09 grahambunce

I solved this by using a custom IEquivalencyStep:

[TestMethod]
public void MyTestMethod()
{
    a.Should().BeEquivalentTo(b, options => options
        .RespectingRuntimeTypes()
        .Using(new TypeEqualityEquivalencyStep()));
}

public class TypeEqualityEquivalencyStep : IEquivalencyStep
{
    public bool CanHandle(IEquivalencyValidationContext context, IEquivalencyAssertionOptions config)
    {
        return true; // Always handle
    }

    public bool Handle(IEquivalencyValidationContext context, IEquivalencyValidator
        structuralEqualityValidator, IEquivalencyAssertionOptions config)
    {
        var subjectType = context.Subject?.GetType();
        var expectationType = context.Expectation?.GetType();

        subjectType?.Should().Be(expectationType, context.Because, context.BecauseArgs);

        if (subjectType?.GetProperties().Any() != true)
        {
            return true; // Don't compare empty class
        }

        return false; // Continue to next step
    }
}

arnileibovits avatar May 05 '20 15:05 arnileibovits

Urs Enzler has an alternative IEquivalencyStep implementation which is a bit more fine grained, see: https://github.com/fluentassertions/fluentassertions/issues/978#issuecomment-441204425


I quite unexpectedly fell into this trap as well - had tests passing which should be failing. We've been using FluentAssertions since v2 or ever v1 and I never really picked up on the importance of this v5 change until I stumbled onto it today by accidence. Tragically, the tests which should have failed have been written 2 years ago and no-one noticed the error until today (this was only possible because the "expected" value is wrong, not the "actual" -- otherwise customers would have complained long ago ;-) ).

I always understood the BeEquivalentTo as a generic way to compare equality without having to implement (and test!) equality on the object manually. Another benefit is the more verbose assertion message which tells you the specific difference, instead of a more generic "it's not the same" (go figure out for yourself how they differ via debugging). So for me were very good reasons for using BeEquivalentTo outside of comparing a source object to it's mapped variant (which, by incident, we don't even care too much about in the case of mapped objects because we're mostly using AutoMapper for that which itself has some functionality to detect if you forget about mapping properties...).

Therefore I'd very much like FluentAssertions to have an equivalency verification in a more stricter sense.


Nitpicking: I don't think having the same property values necessarily constitutes equivalency (in some cases it will). Let me give an example:

Salary { Dollars = 5000 } vs BossSalary { Dollars 5000, Gold = 5 tons }

Pretty sure you wouldn't consider these salaries as equivalent. But might actually consider these two as equivalent:

DollarSalary { Dollars = 5000 } vs EuroSalary { Euros = 4250 } (at the time of writing 5000 USD are worth 4250 EUR).

As such the difference between "BeEqual" and "BeEquivalentTo" is not immediately obvious. It seems much simpler to define equality than equivalency. I'd even go so far as to say one would expect equivalency needing to be tailored to a specific use case.

On a conceptual level I'd prefer assertions to fail more often, and then having to opt in to being less strict. That would save me from making wrong assumptions.


Let me end by saying thank you for the great FluentAssertions library you've provided and maintained for many years now. Also, I've learned a lot from your work. Really appreciate it.

BrunoJuchli avatar Nov 12 '20 15:11 BrunoJuchli

I'm thinking about providing a PR for this. @dennisdoomen @jnyrup Would you be willing to accept one? What are your design ideas on the problem? I'd actually need it in a way that I can not just turn it on/off but also configure the "strictness" of the type equality per type (also see below).

I looked a bit more into this and I've managed to hack this "on top" of FluentAssertions in a way which supports our current requirements but is very hacky. I noticed that in some cases I want to digress from strict type equality, but still want some kind of type equality. This is case when comparing a proxied object with a non-proxied variant. In our case this happens with NHibernate entities, example:

(new FooEntity())
    .Should()
    .BeEquivalentTo(
         session.Load<FooEntity>(id)); // this actually returns a type FooEntityProxy : FooEntity

Guess this is probably also the case for EntityFramework and other ORM's. We also have some comparisons where we compare with a mock (MOQ), same problem here:

new Foo { Nuts = 5 }
    .Should()
    .BeEquivalentTo(
        Mock.Of<Foo>(x => x.Nuts == 5)); // Again, moq creates a sub-class of `Foo`.

Therefore I envision this:

BeEquivalentTo(..., _ => _.WithStrictTypeEquality()) BeEquivalentTo(..., _ => _.IgnoreTypeEquality()) // necessary when WithStrictTypeEquality has been set "by default override"

// The following works a bit similar to .ComparingByMembers<EntityBase>() // on a "first requirement level" it would only influence "WithStrictTypeEquality", not "IgnoreTypeEquality" BeEquivalentTo(..., _ => _.WithTypeEqualityComparisonTypeOverrideFor<T>(t => t.BaseType))

// And maybe, for full customization: BeEquivalentTo(..., _ => _.WithCustomTypeEqualityFor<T>(ITypeEqualityComparer))

BrunoJuchli avatar Nov 13 '20 17:11 BrunoJuchli

Sure. For those people that need this kind of strictness, it might make total sense. I think it all comes down to add the corresponding options on the options object and then build a custom IMatchingRule that only matches if the type matches.

dennisdoomen avatar Nov 13 '20 20:11 dennisdoomen

I've looked into this a bit closer. I see the following obstacles to adding configurability for exact type matching:

  • due to the original intention of the BeEquivalentTo implementation, a member-based comparison fails in case there's no data-members to compare. This doesn't make sense anymore in case the type matches (and doesn't have any members to compare).
  • respecting declared vs runtime types: this is already somewhat confusing. Modifying the existing behavior would be a breaking change. Adding another configuration option in respect to types adds to the confusion.
    • my wish of being able to compare proxies with non-proxy objects, i.E. member-based comparison with non-standard type equality comparison (Foo = FooProxy, but Foo != Bar, Foo != BarProxy, Foo != FooChildProxy) makes it even more complicated. It might also be an argument for modifying the existing behavior instead of adding something separate, because the problems are coupled: when the proxy introduces additional data members, these should be foregone from the member comparison. For example, for comparing Moq-Proxies with "real" objects we have added .Excluding(x => MockType.IsAssignableFrom(x.SelectedMemberInfo.MemberType)) to our default FluentAssertions config. Note: I'm not convinced that this last "requirement" is actually sensible for a broader audience, it's just how our tests currently expect equivalency checks to work.

@dennisdoomen Is my assumption correct that this feature is not important enough to warrant a redesign of the Equivalency comparison config API and implementation?

BrunoJuchli avatar Jan 18 '21 14:01 BrunoJuchli

I've updated my TypeEqualityEquivalencyStep code above to not fail if compared classes are empty.

However, this would be a needed feature in FluentAssertions, perhaps with a separate opt-in (global) setting. Earlier, I always assumed types were compared, until I discovered that many tests were not failing even when they should have been...

arnileibovits avatar Jan 18 '21 16:01 arnileibovits

Is my assumption correct that this feature is not important enough to warrant a redesign of the Equivalency comparison config API and implementation?

I personally don't see enough value in it to redesign the equivalency API for it. But if somebody can provide the necessary PRs to make it possible as an option, I would welcome it and willing to support it thereafter.

dennisdoomen avatar Jan 19 '21 06:01 dennisdoomen

Hi @dennisdoomen I'm glad to find this thread and see our team wasn't the only one running into this. Also glad to see a willingness to add it and support it. We made PR #1704 for this. I'm sure there are plenty of things to correct but we copied the code we have been using for around 3 years now and just refactored to work with version 6.

ryanholden8 avatar Oct 08 '21 14:10 ryanholden8

Yeah, I'd like to see this. Although the current BeEquivalentTo works for me, I know that my method should return a different instance of the same class that has the same data, but should be of the same type. Although it's pretty unlikely my code will somehow return a different class instance with the same properties and data, I still feel slightly uncomfortable that that would be considered "equivalent".

jez9999 avatar Oct 21 '22 11:10 jez9999

I think a good way to do this could be using an internal identifier that allows loose comparisons. For example:

"My string".Should().BeEquivalentTo(It.Should().BeString());

Or even:

"My string".Should().BeEquivalentTo(It.Should().Match(".*string.*"));

It looks ambiguous to other options the package already offer, but the real power of this options appears when you deal with subproperties:

(new {
  SubProp1 = 123,
  SubProp2 = true,
  SubProp3 = DateTime.Now.AddDays(10),
  SubProp4 = "test123"
}).Should().BeEquivalentTo(new {
  SubProp1 = It.Should().BeInteger(),
  SubProp2 = It.Should().BeBoolean(),
  SubProp3 = It.Should().BeDateTime().CloseTo(DateTime.Now.AddDays(10)),
 SubProp4 = It.Should().Match("test.*")
});

This way, you allow a clear and easy to understand way to customize recursive assertions

Farenheith avatar Jan 13 '24 12:01 Farenheith

What would that It return? Some kind of Action<>?

dennisdoomen avatar Jan 13 '24 16:01 dennisdoomen