Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
AxCoder opened this issue Mar 10, 2018 · 23 comments · May be fixed by #1860
Open

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

AxCoder opened this issue Mar 10, 2018 · 23 comments · May be fixed by #1860
Assignees
Milestone

Comments

@AxCoder
Copy link

AxCoder commented Mar 10, 2018

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());
}
@jnyrup
Copy link
Member

jnyrup commented Mar 10, 2018

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
}

@AxCoder
Copy link
Author

AxCoder commented Mar 10, 2018

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);
}

@jnyrup
Copy link
Member

jnyrup commented Mar 10, 2018

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.

@AxCoder
Copy link
Author

AxCoder commented Mar 12, 2018

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)?

@iberodev
Copy link

iberodev commented Sep 6, 2019

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.

@dennisdoomen
Copy link
Member

What's the type of _events?

@iberodev
Copy link

iberodev commented Sep 6, 2019

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.

@grahambunce
Copy link

grahambunce commented Sep 24, 2019

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.

@jnyrup
Copy link
Member

jnyrup commented Sep 24, 2019

@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.

@grahambunce
Copy link

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

@dennisdoomen
Copy link
Member

dennisdoomen commented Sep 24, 2019

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).

@grahambunce
Copy link

grahambunce commented Sep 25, 2019

@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

@arnileibovits
Copy link

arnileibovits commented May 5, 2020

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
    }
}

@BrunoJuchli
Copy link
Contributor

BrunoJuchli commented Nov 12, 2020

Urs Enzler has an alternative IEquivalencyStep implementation which is a bit more fine grained, see:
#978 (comment)


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
Copy link
Contributor

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()
// on a "first requirement level" it would only influence "WithStrictTypeEquality", not "IgnoreTypeEquality"
BeEquivalentTo(..., _ => _.WithTypeEqualityComparisonTypeOverrideFor(t => t.BaseType))

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

@dennisdoomen
Copy link
Member

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.

@BrunoJuchli
Copy link
Contributor

BrunoJuchli commented Jan 18, 2021

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?

@arnileibovits
Copy link

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...

@dennisdoomen
Copy link
Member

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.

@ryanholden8
Copy link

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.

@jez9999
Copy link
Contributor

jez9999 commented Oct 21, 2022

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".

@dennisdoomen dennisdoomen added this to the 7.0 milestone May 29, 2023
@Farenheith
Copy link

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

@dennisdoomen
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.

10 participants