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

CollectionAssert.AreEqual fails for list of lists. #711

Open
kapec94 opened this issue May 3, 2020 · 3 comments
Open

CollectionAssert.AreEqual fails for list of lists. #711

kapec94 opened this issue May 3, 2020 · 3 comments

Comments

@kapec94
Copy link
Contributor

kapec94 commented May 3, 2020

Description

When using CollectionAssert.AreEqual to compare lists of lists (List<List<T>>), the assert fails even though lists in questions are the same and contains the same elements.

Steps to reproduce

Setup: create a MSTest project. Put following code in a sample test:

var list1 = new List<List<int>>
{
    new List<int>{ 1, 2, 3 },
    new List<int>{ 4, 5, 6 }
};
var list2 = new List<List<int>>
{
    new List<int>{ 1, 2, 3 },
    new List<int>{ 4, 5, 6 }
};

CollectionAssert.AreEqual(list1, list2);

Expected behavior

Test passes.

Actual behavior

Test fails with following message:

CollectionAssert.AreEqual failed. (Element at index 0 do not match.)

Environment

Operating system: Windows 10 v1909,
Build version of vstest.console: ???, Package version of MSTest: v2.1.1
framework and adapter: MSTest.TestFramework, MSTest.TestAdapter

Remarks

This issue has its source in private class CollectionAssert.ObjectComparer, which compares objects using object.Equals method. By default, object.Equals checks for reference equality. Since List does not override Equals method, elements of type List<List<T>>, which are List<T>, are compared by ObjectComparer by reference, not by value. Hence the error.

This could be of course an expected behavior, which is fine and understandable. But it should be clearly stated in the AreEqual methods documentation that element comparison is done by invoking object.Equals method. It currently is not; only that it's checked if "collections are equal", which, in this case, I believe to be not true, since the collections checked are in fact equal (not by reference, but by their type and, most importantly, content).

I understand that it would be nearly impossible to correctly implement that assert to be able to handle such use case automatically, so I think that the documentation may be improved here. I could prepare the pull request.

@nohwnd
Copy link
Member

nohwnd commented May 4, 2020

I understand that it would be nearly impossible to correctly implement that assert to be able to handle such use case automatically, so I think that the documentation may be improved here. I could prepare the pull request.

Agree on that, there are libraries such as FluentAssertions that do this better, and from experience I know that recursively comparing objects for equivalency is a huge undertaking that needs a lot of thought, code and options.

So I would also just document this and keep the behavior as is.

@kapec94
Copy link
Contributor Author

kapec94 commented May 4, 2020

Great, I'll prepare the patch... If that's OK? 😉

@nohwnd
Copy link
Member

nohwnd commented May 4, 2020

Sure.

nohwnd pushed a commit that referenced this issue Jun 5, 2020
Added the information that collections' elements are
compared using object.Equals method when custom IComparer
is not specified for the assertion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants