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

Inherited property cannot be excluded or included when comparing interface collections using runtime types #1363

Closed
aayjaychan opened this issue Jul 25, 2020 · 5 comments · Fixed by #1631
Assignees
Labels

Comments

@aayjaychan
Copy link

aayjaychan commented Jul 25, 2020

Description

When a derive class implements an interface through an inherited property, the property cannot be excluded by .Excluding() or included by .Including() when comparing collections of the interface using runtime types.

Complete minimal example reproducing the issue

public interface Interface
{
    int Value1 { get; set; }
    int Value2 { get; set; }
}

public class Base
{
    public int Value1 { get; set; }
    public int Value2 { get; set; }
}

public class Derived : Base, Interface
{
}

[Test]
public void Exclude()
{
    var actual = new Interface[]
    {
        new Derived() { Value1 = 1, Value2 = 2 }
    };
    var expected = new Interface[]
    {
        new Derived() { Value1 = 999, Value2 = 2 }
    };

    actual.Should().BeEquivalentTo(
        expected,
        options => options
            .Excluding(a => a.Value1)
            .RespectingRuntimeTypes());
}

[Test]
public void Include()
{
    var actual = new Interface[]
    {
        new Derived() { Value1 = 1, Value2 = 2 }
    };
    var expected = new Interface[]
    {
        new Derived() { Value1 = 999, Value2 = 2 }
    };

    actual.Should().BeEquivalentTo(
        expected,
        options => options
            .Including(a => a.Value2)
            .RespectingRuntimeTypes());
}

Expected behavior:

Both tests pass

Actual behavior:

Exclude fails with

  Error Message:
   Expected item[0].FromBase to be 999, but found 1.                     

With configuration:                                                      
- Use runtime types and members                                          
- Compare enums by value                                                 
- Include all non-private properties                                     
- Include all non-private fields                                         
- Exclude member root.FromBase                                           
- Match member by name (or throw)                                        
- Be strict about the order of items in byte arrays                      
- Without automatic conversion. 

  Stack Trace:                                                           
     at FluentAssertions.Execution.LateBoundTestFramework.Throw(String message)                                                                   
   at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message)                                                                      
   at FluentAssertions.Execution.CollectingAssertionStrategy.ThrowIfAny(IDictionary`2 context)                                                    
   at FluentAssertions.Equivalency.EquivalencyValidator.AssertEquality(EquivalencyValidationContext context)                                      
   at FluentAssertions.Collections.CollectionAssertions`2.BeEquivalentTo[TExpectation](IEnumerable`1 expectation, Func`2 config, String because, Object[] becauseArgs)

Include fails with

  Error Message:
   System.InvalidOperationException : No members were found for comparison. Please specify some members to include in the comparison or choose a more meaningful assertion.                                                
  Stack Trace:                                                           
     at FluentAssertions.Equivalency.GenericEnumerableEquivalencyStep.Handle(IEquivalencyValidationContext context, IEquivalencyValidator parent, IEquivalencyAssertionOptions config)
   at FluentAssertions.Equivalency.EquivalencyValidator.RunStepsUntilEquivalencyIsProven(IEquivalencyValidationContext context)
   at FluentAssertions.Equivalency.EquivalencyValidator.AssertEqualityUsing(IEquivalencyValidationContext context)
   at FluentAssertions.Equivalency.EquivalencyValidator.AssertEquality(EquivalencyValidationContext context)
   at FluentAssertions.Collections.CollectionAssertions`2.BeEquivalentTo[TExpectation](IEnumerable`1 expectation, Func`2 config, String because, Object[] becauseArgs)

Versions

FluentAssertions 5.10.3 and 6.0.0-alpha.1
NUnit 3.12.0
.NET Core 3.1

@dennisdoomen
Copy link
Member

Thank you for reporting this. I've managed to reproduce it. I'm afraid it's a bit of a compiler/language issue. Because your expectation is of type Interface, your basically telling FA to exclude Value2 that is declared by that interface. But the property your setting is the one from the Base class. At first I thought it's FA not doing what is supposed to do, but then I noticed how Rider marks the Value2 property of the interface as unused.

image

So it looks like your Derived class hides some property. The current behavior was made strict after somebody reported #956

@aayjaychan
Copy link
Author

Value2 is marked as unused probably because it is only used in Include(), which doesn't seem to be in the code in the image.

Looking at #956 at its fix, I think the member selection of FluentAssertions before the fix was too lax, so it picked up members not selected in the expression. But after the fix, the selection became too strict, and this issue surfaced.

I think the problem is that MemberPath is constructed using MemberInfo.DeclaredType, so it loses type information on the expectation and is oblivious to the fact that the inherited properties are part of the interface in the derived class. Preserving the original type by using .ReflectedType instead of .DeclaredType when selecting members to compare might fix this issue.

@dennisdoomen
Copy link
Member

Value2 is marked as unused probably because it is only used in Include(), which doesn't seem to be in the code in the image.

Actually, it's used in the object initializer twice.

Preserving the original type by using .ReflectedType instead of .DeclaredType when selecting members to compare might fix this issue.

You might be on to something.

@dennisdoomen
Copy link
Member

I did some experimenting the last two days, but fixing this is going to be a lot harder than I thought. Just switching to the ReflectedType fixes this particular scenario, but starts to break a lot of other cases (especially around "covariant" and new properties`). I think there's something fundamentally wrong in the way we iterate the properties of a type, and I'm not sure we can fix this without a lot of breaking changes.

@dennisdoomen
Copy link
Member

Fixed through #1631

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