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

ExcludingNestedObjects option does not behave as expected #2210

Closed
andrewslavin opened this issue May 26, 2023 · 8 comments
Closed

ExcludingNestedObjects option does not behave as expected #2210

andrewslavin opened this issue May 26, 2023 · 8 comments

Comments

@andrewslavin
Copy link

andrewslavin commented May 26, 2023

Description

ExcludingNestedObjects option does not behave as expected.

Reproduction Steps

class Parent
{
    public int Id { get; set; }
    public Child Child { get; set; }
}

class Child
{
    public int Id { get; set; }
}

var p1 = new Parent { Id = 0, Child = new Child { Id = 1 } };
var p2 = new Parent { Id = 0, Child = new Child { Id = 2 } };

p1.Should().BeEquivalentTo(p2, o => o.ExcludingNestedObjects());

Expected behavior

Assertion passes, because help text for ExcludingNestedObjects says Causes the structural equality check to exclude nested collections and complex types, and Child is a complex type.

Actual behavior

Assertion fails with the message

Expected property p1.Child to be Child
{
    Id = 2
}, but found Child
{
    Id = 1
}

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@dennisdoomen
Copy link
Member

This is by design. What you're saying with ExcludingNestedObjects is that FA should not traverse the properties of the object returned by the Child property and instead fall back on a basic Equals call. Since the default implementation of Equals on a class is doing a reference equality, it fails.

@andrewslavin
Copy link
Author

andrewslavin commented May 29, 2023

Thanks @dennisdoomen, that makes sense. Two suggestions then:

  1. Improve ExcludingNestedObjects help text to better explain how exactly complex properties will be compared.
  2. Add a new option, that will allow to exclude complex properties completely, not just from structural equality check.

@dennisdoomen
Copy link
Member

Add a new option, that will allow to exclude complex properties completely, not just from structural equality check.

What do you mean with that?

@andrewslavin
Copy link
Author

I was going to reply

Add something like ExcludeComplexProperties, which will completely ignore complex properties when comparing expected and actual, like I can completely ignore one property now using Excluding.

But then I read help text for Excluding and it says Excludes the specified (nested) member from the structural equality check. So it doesn't exclude the member completely, it only switches it from structural check to basic Equals call? Then I do not understand why in my test

p1.Should().BeEquivalentTo(p2, o => o.ExcludingNestedObjects());

fails, but

p1.Should().BeEquivalentTo(p2, o => o.Excluding(p => p.Child));

succeeds.

@dennisdoomen
Copy link
Member

Excluding completely excludes the property or field. ExcludingNestedObjects doesn't exclude the property or field, but doesn't do a recursive comparison on the properties of fields of the object returned by that property.

@andrewslavin
Copy link
Author

andrewslavin commented May 30, 2023

Excluding completely excludes the property or field. ExcludingNestedObjects doesn't do a recursive comparison.

Yes that's what I got in my tests. I think documentation needs some improvement because descriptions for both options talk about "structural" equality only:

  • ExcludingNestedObjects: Causes the structural equality check to exclude nested collections and complex types.
  • Excluding: Excludes the specified (nested) member from the structural equality check.

Description for Excluding should not say "structural", and description for ExcludingNestedObjects should explain better what "structural equality" means. I do not think people pay attention to the word "structural" and presume that properties will be just excluded from equality check. That's what I presumed.

On a separate note, would it be feasible to add smth like ExcludeComplexTypes, which will completely exclude all complex properties and fields?

@dennisdoomen
Copy link
Member

Yes that's what I got in my tests. I think documentation needs some improvement because descriptions for both options talk about "structural" equality only:

I did that in #2211

Description for Excluding should not say "structural", and description for ExcludingNestedObjects should explain better what "structural equality" means. I do not think people pay attention to the word "structural" and presume that properties will be just excluded from equality check. That's what I presumed.

We have extensive documentation on this structural comparison here. Although I do agree that the documentation and the code uses different terminology. That's something for v7 to fix.

@dennisdoomen
Copy link
Member

On a separate note, would it be feasible to add smth like ExcludeComplexTypes, which will completely exclude all complex properties and fields?

That would be an entirely new feature. Please raise a separate feature request for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants