record struct with List or IEnumerable property fails Should BeEquivalentTo

[kijanawoodard]

Before you file a bug, have you:

• Tried upgrading to newest version of Fluent Assertions, to see if your issue has already been resolved and released? Yes
• Checked existing open and closed issues, to see if the issue has already been reported? Yes
• Tried reproducing your problem in a new isolated project? Yes
• Read the documentation? Yes
• Considered if this is a general question and not a bug?. For general questions please use StackOverflow.

Description

A record struct which contains a List or IEnumberable fails BeEquivalentTo

Complete minimal example reproducing the issue

using System.Collections.Generic;
using FluentAssertions;
using Xunit;

namespace fluent_assertion_record_struct_issue;

record struct Foo(List<int> Values);

[Fact]
public void TestRecordStructWithLists()
{
    var expected = new Foo(new() {1, 2, 3});
    var actual = new Foo(new() {1, 2, 3});
    actual.Should().BeEquivalentTo(expected);
}

Expected behavior:

The test should pass, by default, just like with a record / record class

Actual behavior:

Xunit.Sdk.XunitException: Expected actual to be Foo { Values = System.Collections.Generic.List`1[System.Int32] }, but found Foo { Values = System.Collections.Generic.List`1[System.Int32] }.

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
- Match member by name (or throw)
- Be strict about the order of items in byte arrays
- Without automatic conversion.

   at FluentAssertions.Execution.XUnit2TestFramework.Throw(String message) in /_/Src/FluentAssertions/Execution/XUnit2TestFramework.cs:line 33
   at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message) in /_/Src/FluentAssertions/Execution/TestFrameworkProvider.cs:line 32
   at FluentAssertions.Execution.CollectingAssertionStrategy.ThrowIfAny(IDictionary`2 context) in /_/Src/FluentAssertions/Execution/CollectingAssertionStrategy.cs:line 42
   at FluentAssertions.Equivalency.EquivalencyValidator.AssertEquality(Comparands comparands, EquivalencyValidationContext context) in /_/Src/FluentAssertions/Equivalency/EquivalencyValidator.cs:line 27
   at FluentAssertions.Primitives.ObjectAssertions`2.BeEquivalentTo[TExpectation](TExpectation expectation, Func`2 config, String because, Object[] becauseArgs) in /_/Src/FluentAssertions/Primitives/ObjectAssertions.cs:line 145
   at FluentAssertions.Primitives.ObjectAssertions`2.BeEquivalentTo[TExpectation](TExpectation expectation, String because, Object[] becauseArgs) in /_/Src/FluentAssertions/Primitives/ObjectAssertions.cs:line 97

Versions

• Which version of Fluent Assertions are you using? 6.5.1
• Which .NET runtime and version are you targeting? .net 6

Additional Information

record and class works fine.
This is related to #1507 and #1451.

I tried options => options.ComparingRecordsByMembers(), but that did not work.
options => options.ComparingByMembers<Foo>() does work.

[dennisdoomen]

I suspect FA treats the record struct as a struct and thus as a value type.

[kijanawoodard]

I suspect the same.
I propose that FA, for the purpose of making assertions in tests, should treat them like record (aka record class).

[salvois]

Hi @kijanawoodard, @dennisdoomen,
in my understanding, there is no objective way to detect whether a type is a record struct in TypeExtensions.IsRecord. This seems to be confirmed here: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-10.0/record-structs#open-questions

In your opinion, could FA try a best guess based on some clues? Having System.ValueType as base type is of course necessary, but how wrong could FA be if it finds a compiler generated op_Inequality and op_Equality, for example? I appreciate that they may be manually overridden, but I have a gut feeling that we wouldn't want FA to do member comparison in this case anyways.

What do you think?
Thanks,
Salvo

[salvois]

Hi @dennisdoomen, @kijanawoodard,
I have filed pull request #2009 leveraging the above best guess, hoping you may find it useful.
Thanks,
Salvo