Object Graph Comparison - ShouldBeEquivalentTo(...)

[TaffarelJr]

I took a stab at implementing object graph comparison, noted in issue #370. I've done similar stuff in other projects, and thought I could knock it out. Let me know what you think.

For any two objects (actual & expected), the process is:

• First check for null values.
  • Both null, then exit.
  • Only one null, throw exception.
• Then compare data types.
  • Different, throw exception.
• Now compare values according to data type (in order of precedence)
  • ValueType
    • Compare using Object.Equals().
  • String
    • Compare using StringComparison.Ordinal.
  • IEnumerable (any list)
    • Loop through each item and start new comparison. (recursion)
  • <else>
    • Use reflection to get all public, non-static properties of the data type.
    • Loop through each property and start new comparison. (recursion)

Throughout the recursion process, it also keeps track of the current path, so if an error occurs somewhere in the object graph it appears in the error message. The format ends up looking like this:

<root object> [<data type>]
    <property> [<data type>]
        Element [<index>] [<data type>]
            ...

    Expected value to be
<expected>
    but was
<actual>

[JakeGinnivan]

This is a great start.

I also think we need to keep a list of seen objects to prevent infinite recursion bugs.

Also worth reading the discussion at #245, there was some thought put into the API and it's feature set

[josephwoodward]

@JakeGinnivan Are you happy for this to be merged in its current state? If any issues or edge cases arise then we can tackle those separately but it seems like a great addition to Shouldly.

[urig]

@JakeGinnivan @josephwoodward This is awesome and I need it. Thanks for working on it! Would love to help - please let me know if I can.

[josephwoodward]

I'd really like to get this merged too @urig, though I'm currently struggling with migrating from project.json to .csproj (see this PR) - I think it's best we migrate to the new tooling before merging anymore changes.

Any assistance there would always be welcome, it's almost there, just ran into some stumbling blocks.

[urig]

Hi. I have very little experience with Core at this time so I'm doubtful I can help. I will try though :)

[TaffarelJr]

Sorry, I meant to pick back up on the comments made by @JakeGinnivan , but you know how life goes. :) I'll see if i can get back to it this week.

[josephwoodward]

@TaffarelJr Interested to know why you closed this? I was planning on merging it once the tooling changes were completed?

[TaffarelJr]

Hmm, TBH I'm not sure how this got closed. I think in pulling down your latest changes and rebasing my stuff on top of it, I may have done something that cut the connection. I'm almost done with my changes though, and I'll be putting this back up in a new PR soon.

[josephwoodward]

@TaffarelJr That would be amazing, was looking forward to getting it merged as part of the official 3.0 release, it's a feature loads of people have been asking for :)

[TaffarelJr]

Alright, I think I've got it now. It's a bit cleaner than it was before, and it now keeps track of which objects it's compared along the way. If it detects that it's already compared two objects (anywhere in the object graph), then it skips over them. This should keep us from having infinite loops.

[josephwoodward]

Awesome! Thanks so much for this @TaffarelJr !

[Ulriksen]

As #432 has been merged (ref #411 (comment)), an chance this PR will be merged soon?

[einarwh]

As someone who has implemented something similar on our project, it would be very nice to see something like this included in Shouldly. However, avoiding infinite regress is a tricky business, since the object graph could conceivably grow to infinite depth.

public class ZenoScenario
{
  [Fact]
  public void Test()
  {
    new Turtle().ShouldBeEquivalentTo(new Turtle());
  }

  internal class Turtle
  {
    public Turtle Next
    {
      get { return new Turtle(); }
    }
  }
}

(I discovered this problem because my own implementation blew up during similar conditions.)

[TaffarelJr]

Right, it's probably not unbreakable code at this point. :) While I put in a reasonable effort to prevent infinite loops (and I welcome improvements), I don't think we can entirely prevent them here. And ultimately I think that's OK in this case, because it's not meant to be mission-critical code. If a developer uses .ShouldBeEquivalentTo(...) in a situation that triggers an infinite loop, well, then they've probably got a situation that's too complex for this shortcut. :)

[einarwh]

Yeah I think you're right. While it's nice to have very robust code, it's probably a case of diminishing returns on investment in trying to solve an impossible problem. You can always construct infinite object graphs, and there's not much to be done about them.

Another (silly) one:

public class NeverendingScenario
{
    [Fact]
    public void Test()
    {
        new Limahl().ShouldBeEquivalentTo(new Limahl());
    }

    internal class Limahl
    {
        public IEnumerable<string> Story
        {
            get
            {
                while (true)
                {
                    yield return "Reach the stars";
                    yield return "Fly a fantasy";
                    yield return "Dream a dream";
                    yield return "And what you see will be";
                }
            }
        }
    }
}

[einarwh]

One thing you could consider is try to detect problems and terminate the tests a bit more gracefully. For instance, since you are already keeping track of the property path, you could check if the path got unreasonably deep and fail the test at that point. Similarly you could choose some arbitrary cap on the number of elements in an IEnumerable<T> and do a check like elements.Skip(maxElementCount).Any() instead of failing with an OutOfMemoryException when trying to convert an infinite sequence into a list. I have tests like these:

[Fact]
public void InfiniteTurtlesShouldNotBeLikeThemselvesBecauseTheyAreInfinite()
{
    var ex = Assert.Throws<ShouldAssertException>(() =>
    {
        new Turtle().ShouldBeLike(new Turtle());
    });

    ex.Message.ShouldStartWith("Gave up comparing object graphs because the trail got deeper than max depth");
}

[Fact]
public void LimahlShouldNotBeLikeHimselfBecauseTheStoryIsNeverending()
{
    var ex = Assert.Throws<ShouldAssertException>(() =>
    {
        new Limahl().ShouldBeLike(new Limahl());
    });

    ex.Message.ShouldStartWith(
        "Gave up comparing enumerables because the count exceeded max number of elements");
}

It's not perfect, but it tells the user what and where the problem was.

[Rookian]

What is the status of this PR? When can we use such an essential functionality?

[natiki]

Perhaps using a 3rd party solution is an option to get this implemented? For instance: https://github.com/GregFinzer/Compare-Net-Objects

[mikesigs]

Any traction on this? It's the only thing Shouldly is missing imo!

[SamerAdra]

Watching this. Once it gets merged I will gladly switch to Shouldly.

[dgroh]

+1

[alansingfield]

I took a slightly different approach to this problem by splitting out the object graph into multiple shouldly assertions, see UnitTestCoder

This is an example of what UnitTestCoder will automatically generate for you at runtime based on a populated model, copy the code back into your unit test and you have 100% coverage!

model.Customers.Count().ShouldBe(2);
model.Customers[0].Name.ShouldBe("Apple");
model.Customers[0].Invoices.Count().ShouldBe(2);
model.Customers[0].Invoices[0].Amount.ShouldBe(123.45m);
model.Customers[0].Invoices[1].Amount.ShouldBe(6464.55m);
model.Customers[1].Name.ShouldBe("Facebook");
model.Customers[1].Invoices.Count().ShouldBe(1);
model.Customers[1].Invoices[0].Amount.ShouldBe(123.45m);

It avoids circular references by keeping track of objects it has already seen and referring back to them by array index.

[dgroh]

@alansingfield I would not encourage you using this pattern, rather try this:

            listOfElements.TrueForAll(x =>
            {
                return expectedListOfElements.Exists(
                    y => y.Id == x.Id &&
                    y.Map.Id == x.MapId &&
                    y.LabelText == x.LabelText &&
                    y.Location == x.Location &&
                    y.Points == x.Points &&
                    y.StatusTextOffset == x.StatusTextOffset &&
                    y.Type == x.Type
                );

            }).ShouldBeTrue();

[josephwoodward]

Going to be merging this one as soon as I've fixed #524

[mikesigs]

@alansingfield What @dgroh said. But also, wrap all your assertions inside an x.ShouldSatisfyAllConditions and you're good.

[martincostello]

Going to be merging this one as soon as I've fixed #524

That looks fixed now?

[mikesigs]

@josephwoodward ♫ All I want for Christmas is this PR merged. This PR merged. This PR merged. ♫

[josephwoodward]

@martincostello @mikesigs Going to be merging this along with the ShouldMatchApproved xplat support in the new few days as a beta. Having been playing with this PR for a bit there's a number of edge cases that aren't right, but it's a good start.

[thomas-parrish]

What is the hold up here?

[josephwoodward]

@thomas-parrish Been doing a bit of extra work around this, want this to be part of 4.0 (and hopefully get a beta out that we can iterate on for a few versions), it's good to merge now though and add the extra bits as a PR then get a beta out.

[JakeGinnivan]

Woot

[Rookian]

4.0.0-beta0001 still has no ShouldBeEquivalentTo method? Any progress?

[mikesigs]

Missed it by that much! The 4.0.0-beta0001 tag was created on the commit juuuust before this one was merged. My guess is we'll get it on the next beta release. The fact that is was merged is a really good sign!

[josephwoodward]

Yes that' right, I've been making a few changes for it for the next beta release which should be out shortly.

[mikesigs]

Bump :)

[thomas-parrish]

3 years and counting... :\

[josephwoodward]

I know, been sorting out the next release ready for this week so watch this space. It's really early implementation at the moment so will leave the the package as a beta for a while.

[alansingfield]

Hello Joseph, I understand, It's harder than it looks... I've been updating UnitTestCoder.Shouldly to take account of more data types, enumerables and the like - but there's still plenty more it could do.

You either have to limit yourself to the simplest POCO objects, or have to make decisions on which properties to follow and which to ignore. I took the route of adding a nofollow parameter so you can tell the system when to stop digging. I'm intending to turn this into a more fluent interface when I get the time.

Are you intending to pass in another object of the same type, or allow any anonymous type with matching properties to work?

MyModel obj = MakeModel();
// Compare to anonymous type
obj.ShouldBeEquivalentTo(new { Product = "ABC", Price = 234.56m });

MyModel obj = MakeModel();
// Compare to another instance; but - what if it's a non-constructable type?
MyModel comparison = new MyModel()
{
    Product = "ABC",
    Price = 234.56m
};

obj.ShouldBeEquivalentTo(comparison);

The other issue is how to determine equivalence. For primitive types and strings, it's probably OK, but once you get circular referenced objects, how do you match from the comparison to the target? I took the approach of using the ordinal, a little like an MVC form submission. But this procludes you from testing plain IEnumerable<>, you need an IList<>.

model.Customers[0].Invoices[1].Customer.ShouldBe(model.Customers[0]);

I'll be interested to see what you've done and can if I can contribute anything useful I will!

[josephwoodward]

This change has now been released as part of the 4.0.0-beta0002 release which is now available on NuGet.

This feature is still in its infancy and there are a few cases I've noticed it doesn't cater for, so I'll leave it in beta for a while so we can get some issues posted/fixed around it before we release a 4.0 RC The long term plan is to add some configuration options so you can cusrtomise how the comparison behaves.

In the meantime give it a try, and if anyone notices any issues with this new feature then we'd appreciate it if you could report them by opening a new issue.

[josephwoodward]

@alansingfield Those are some interesting questions and to be honest, I don't know the answer right now. As it seems there's no right or wrong way for this to behave as default, I've been looking at a lot of other object comparison libraries out there to get an idea of what they have set as defaults.

The long term plan (as mentioned above) is to provide the ability to configure how ShouldBeEquivalentTo behaves under the different scenarios you've highlighted.

[PureKrome]

Not sure if this is late to the party OR totally wrong answer, here ...

but .. for comparing two .net poco's, I just JSON serialize both the current poco and the expected poco and then compare two strings.

Here is the library code.

This is comparing the data values of each poco, not memory addresses etc.

e.g. (testing two pocos)

var actual = new FakeFoo();
var expected = new FakeFoo();

actual.ShouldLookLike(expected); // No exception Thrown.

e.g. (two pocos are different)

// Arrange.
var actual = new FakeFoo();
var expected = new FakeFoo
{
    Id = 1
};

// Act.
var exception = Should.Throw<Exception>(() => actual.ShouldLookLike(expected));

// Assert.
exception.ShouldNotBeNull();