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

Excluding properties behind an array doesn't work in BeEquivalentTo on collections #1919

Closed
Turnerj opened this issue May 3, 2022 · 10 comments · Fixed by #2135
Closed

Excluding properties behind an array doesn't work in BeEquivalentTo on collections #1919

Turnerj opened this issue May 3, 2022 · 10 comments · Fixed by #2135
Assignees
Labels

Comments

@Turnerj
Copy link

Turnerj commented May 3, 2022

Description

A bit of a weird title but basically if I call myCollection.Should().BeEquivalent(expected, cfg => cfg.Excluding(i => i.SomeArray[0].SomeProperty)), the property isn't correctly excluded and I get the follow assertion error:

Expected property root[0].Items[0].Name to be "Foo", but "Bar" differs near "Bar"

Complete minimal example reproducing the issue

var expected = new A[] {
	new A { Items = new List<B> { new B { Name = "Foo" } } }
};

var actual = new A[] {
	new A { Items = new List<B> { new B { Name = "Bar" } } }
};

actual.Should().BeEquivalentTo(
	expected,
	cfg => cfg.Excluding(a => a.Items[0].Name)
);

class A
{
	public string Name { get; set; }
	public List<B> Items { get; set; }
}

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

Expected behavior:

The excluding rule to be used and the test to pass.

Actual behavior:

The excluding rule is ignored and the test fails.

Versions

  • Which version of Fluent Assertions are you using?
    v6.6.0

  • Which .NET runtime and version are you targeting? E.g. .NET framework 4.6.1 or .NET Core 2.1.
    .NET 6

Additional Information

I believe this is a regression between v5.5.3 and v6.6.0 - I haven't (yet) nailed down which version but will update when I have.

Edit: The regression starts in v6.0.0. The last v5 has this behaving correctly.

Additionally, this only seems to affect properties behind the array in the excluded check. Properties on the same level as the array work as expected if excluded.

@IT-VBFK
Copy link
Contributor

IT-VBFK commented May 4, 2022

Ahhm... this should be fixed with the next release. See #1782

The new syntax will be:

actual.Should().BeEquivalentTo(
	expected,
	cfg => cfg
                 .For(a => a.Items)
                 .Exclude(b => b.Name));

@Turnerj
Copy link
Author

Turnerj commented May 4, 2022

Ahhh OK. I did look for existing issues about this but must not have had the right keywords or something.

Looking forward to the next release then!

@Turnerj Turnerj closed this as completed May 4, 2022
@Turnerj
Copy link
Author

Turnerj commented May 5, 2022

I cloned the develop branch and tested this (as that PR you referenced was merged) but it doesn't seem to work:

var expected = new A[]
{
    new A { Items = new List<B> { new B { Name = "Foo" } } }
};

var actual = new A[]
{
    new A { Items = new List<B> { new B { Name = "Bar" } } }
};

actual.Should().BeEquivalentTo(
    expected,
    cfg => cfg.For(a => a.Items).Exclude(b => b.Name)
);

This is the exception the test threw:

Expected property actual[0].Items[0].Name to be "Foo", but "Bar" differs near "Bar" (index 0).

With configuration:
- Use declared types and members
- Compare enums by value
- Compare tuples by their properties
- Compare anonymous types by their properties
- Compare records by their members
- Include non-browsable members
- Include all non-private properties
- Include all non-private fields
- Exclude member Items[]Name
- Match member by name (or throw)
- Be strict about the order of items in byte arrays
- Without automatic conversion.

@Turnerj Turnerj reopened this May 5, 2022
@Turnerj
Copy link
Author

Turnerj commented May 5, 2022

If I was to have a guess at the problem, one difference between my example and the linked PR is that my base type I run Should() on is an array. I don't know the inner workings of Fluent Assertions to know whether that would make a difference but I think it interferes with the member path checks (eg. actual[0].Items[0].Name != Items[0].Name).

@ITaluone
Copy link
Contributor

ITaluone commented May 5, 2022

Hmm.. this have to be a bug then..
@whymatter

@whymatter
Copy link
Contributor

whymatter commented May 5, 2022

My PR was about the For and Excluding syntax to make an exclusion for all items in an IEnumerable. The initial issue here was about excluding a specific array index. These are two different 'exclusion types'.

For my understanding, the following should work on the 6.6.0 release

actual.Should().BeEquivalentTo(
	expected,
	cfg => cfg.Excluding(a => a.Items[0].Name)
);

@ITaluone
Copy link
Contributor

ITaluone commented May 5, 2022

Hmm... but what if I want to exclude Name on every Item? IMHO this is what @Turnerj wants to acheive here..

@Turnerj
Copy link
Author

Turnerj commented May 5, 2022

Technically it is both. The code I've inherited uses Items[0].Name in the syntax @whymatter mentions because it wants to check only one item but that is broken as of v6.0.0 according to my testing of different Fluent Assertion versions. That said, in this code I've inherited, there is only one item in that array so it really is all items.

I think the underlying issue of why Items[0].Name doesn't work is why For(a => a.Items).Excluding(b => b.Name) doesn't work - there is something special about the root object being an array (my actual and expected values).

@whymatter
Copy link
Contributor

whymatter commented May 5, 2022

Hmm... but what if I want to exclude Name on every Item? IMHO this is what @Turnerj wants to acheive here..

My point is that even the existing functionality (excluding specific indices) before my PR is not working. Which I think is strange, I guess there are unit tests for this..

@dennisdoomen
Copy link
Member

I initially thought it worked given we have this successful test, but that one is not using a collection at the root. Also, I can confirm the behavior is slightly different when the root is a collection.

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

Successfully merging a pull request may close this issue.

5 participants