BeEquivalentTo does not exclude internal members

[comecme]

BeEquivalentTo fails if an internal property has a different value.

The documentation, section "Including properties and/or fields" states:

Barring other configuration, Fluent Assertions will include all public properties and fields

However, it also seems to take into account all internal properties and fields.

Who is wrong? The documentation or the implementation? What is BeEquivalentTo supposed to do?

In the following code sample, PrivateMembersShouldNotBeCompared succeeds but InternalMembersShouldNotBeCompared fails on the compare of property InternalProperty.

using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace UnitTestProject
{
    [TestClass]
    public class TestBeEquivalentTo
    {
        class TestSubject
        {
            private string PrivateProperty { get; }
            internal string InternalProperty { get; }
            public string PublicProperty { get; }

            public TestSubject(string privateString, string internalString, string publicString)
            {
                PrivateProperty = privateString;
                InternalProperty = internalString;
                PublicProperty = publicString;
            }
        }

        [TestMethod]
        public void PrivateMembersShouldNotBeCompared()
        {
            TestSubject first = new TestSubject("A", "A", "A");
            TestSubject second = new TestSubject("B", "A", "A");

            first.Should().BeEquivalentTo(second);
        }

        [TestMethod]
        public void InternalMembersShouldNotBeCompared()
        {
            TestSubject first = new TestSubject("A", "A", "A");
            TestSubject second = new TestSubject("B", "B", "A");

            first.Should().BeEquivalentTo(second);
        }
    }
}

[dennisdoomen]

Are you using the [InternalsVisible] attribute?

[comecme]

Not in the posted sample code, but I do use it in my real code.

Seems like you are saying that BeEquivalentTo examines all properties and fields visible to the test project. If that's true, you might want to add something about that to the documentation. It's basically just public members, but if internal members are visible to the test class, internal properties will be compared too.

I would like to be able to set an option to include only public members.

[dennisdoomen]

That's the most likely reason. AFAIK, the reflection API does not make a distinction between public and internal.

[dennisdoomen]

Hmm, I've just ran a test and can confirm this is a bug. FA treats internal properties as public.

[Fact]
public void When_scenario_it_should_behavior()
{
    new Foo
    {
        InternalProperty =  "aa",
        PublicProperty =  "pp"
    }.Should().BeEquivalentTo(new Foo
    {
        InternalProperty = "bb",
        PublicProperty = "pp"
    });
}

public class Foo
{
    internal string InternalProperty { get; set;}
    public string PublicProperty { get; set;}
}

After I've changed the implementation of TypeExtensions.GetPublicProperties to

private static PropertyInfo[] GetPublicProperties(Type type)
{
    return type.GetProperties(PublicMembersFlag).Where(pi =>
        pi.GetGetMethod() != null && !pi.GetGetMethod().IsFamily).ToArray();
}

The test passed.

cc @jnyrup

[jnyrup]

Might be related to #744

[comecme]

I would still prefer to be able to choose whether I want to include internal properties.

Don't know what default behavior I would expect. To prevent a breaking change it might be best to compare both public and internal by default, and introduce an option to exclude internal.

By the way: I'd expect the code to only compare internal properties if they are visible (i.e. defined in the same project or exposed using InternalsVisibleTo.

[nielsheeren]

I found a work-around that only checks public properties:

using FluentAssertions.Equivalency;
...
result.Should().BeEquivalentTo(expected, options => options
    .Including(info => info.WhichGetterHas(FluentAssertions.Common.CSharpAccessModifier.Public)));

[comecme]

@jnyrup Is something preventing the solution posted by @dennisdoomen to be implemented in FA? Or do I just have to wait till FA 6.0 comes out?

[dennisdoomen]

Time ;-)