nunit: Has.Property cannot see explicit interface implementation properties

Using nunit.framework 3.2 and nunit3-console 3.0.1.

Code:

using NUnit.Framework;

namespace NUnitHasProperty
{
    public interface IFoo
    {
        string Description { get; }
    }

    public class ImplicitFoo : IFoo
    {
        public string Description => "Foo";
    }

    public class ExplicitFoo : IFoo
    {
        string IFoo.Description => "Foo";
    }

    [TestFixture(typeof(ImplicitFoo))]
    [TestFixture(typeof(ExplicitFoo))]
    public class HasPropertyTestCase<TFoo>
        where TFoo : IFoo, new()
    {
        [Test]
        public void DescriptionIsFoo()
        {
            IFoo thing = new TFoo();
            Assert.That(thing, Has.Property(nameof(IFoo.Description)).EqualTo("Foo"));
        }
    }
}

Results:

NUnit Console Runner 3.0.5813
Copyright (C) 2015 Charlie Poole

Runtime Environment
   OS Version: Microsoft Windows NT 10.0.10240.0
  CLR Version: 4.0.30319.42000

Test Files
    NUnitHasProperty.dll


Errors and Failures

1) Error : NUnitHasProperty.HasPropertyTestCase<ExplicitFoo>.DescriptionIsFoo
System.ArgumentException : Property Description was not found
Parameter name: name
   at NUnit.Framework.Constraints.PropertyConstraint.ApplyTo[TActual](TActual actual)
   at NUnit.Framework.Assert.That[TActual](TActual actual, IResolveConstraint expression, String message, Object[] args)
   at NUnit.Framework.Assert.That[TActual](TActual actual, IResolveConstraint expression)
   at NUnitHasProperty.HasPropertyTestCase`1.DescriptionIsFoo() in C:\temp\NUnitHasProperty\NUnitHasProperty\HasPropertyTestCase.cs:line 29

Run Settings
    WorkDirectory: C:\temp\NUnitHasProperty\NUnitHasProperty\bin\Debug
    NumberOfTestWorkers: 12

Test Run Summary
    Overall result: Failed
   Tests run: 2, Passed: 1, Errors: 1, Failures: 0, Inconclusive: 0
     Not run: 0, Invalid: 0, Ignored: 0, Explicit: 0, Skipped: 0
  Start time: 2016-04-06 04:58:54Z
    End time: 2016-04-06 04:58:54Z
    Duration: 0.085 seconds

Results (nunit3) saved as TestResult.xml

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Comments: 39 (38 by maintainers)

Commits related to this issue

Most upvoted comments

Agree on the terminology. In addition, I believe the correct usage for class : interface is that the class “implements” the interface. At least that’s what I was taught to say. 😄

@jnm2 ok, thanks for edit

By saying “anybody eager to change that” I meant: I can contribute here as long as you have consensus on choosing the approach aligned with your taste, so that this work will not go in vain 😃

@nunit/framework-team what do you think?

I’m not in the team but I like the idea. Anybody eager to change that ?

@NUnitTester Yep, that’s the point I’m making when I say we should use typeof(T) to search for properties, which will be the interface type if the compile-time type of the actual expression passed to Assert.That was the interface type.

At least I think it’s the same point. I would not say that we should find an explicit implementation in any case. Only in the case where the compile-time type of the actual expression is the interface type. Same as how C# will not find an explicit implementation unless you first cast to the interface type. So Assert.That<Array> should not find a Count property, but Assert.That<ICollection> should.

I think there is a big difference if we talk about interfaces and abstract classes or implementation classes.

The object is used by interface in this example. And the interface has such a property, so it must be found in any case, explicit or implicit. It is part of the interface.

This is the same thing, if IReadOnlyCollection with Count-Property is uses, but the implementing class is an Array with “normal” Property Length.

I’d like to propose that we search for properties on typeof(T), where T is the generic type parameter of Assert.That<T>. Usually the type argument is implicit when calling Assert.That. In the initial examples, T would be IFoo because the initial example is implicitly calling Assert.That<IFoo>. This strategy would make the initial example work and fit most people’s expectations, I think.

We probably don’t want to stop our current search for properties on actual.GetType(). If we stopped searching on actual.GetType(), it would break tests like this:

Base actual = new Derived(); // This code may have been spread out in reality
Assert.That(actual, Has.Property("SomeProperty").EqualTo(42));

class Base { }
class Derived : Base
{
    public int SomeProperty { get; set; }
}

I do not like the idea of being able to find explicitly-implemented properties when Assert.That<object> or Assert.That<ImplementingType> is called. I think that you should implicitly or explicitly use Assert.That<InterfaceType> if you want to use explicitly-implemented properties, just like how C# and VB require you to cast to the interface type to access them.

An outstanding question would be whether to prefer typeof(T) properties with the same name over actual.GetType() properties. The most backwards-compatible answer would be to prefer what we have done in the past and add typeof(T) as a fallback if no property was found. It seems unlikely for the difference to matter in practice though. I lean towards preferring typeof(T) over actual.GetType() because that’s the property the compiler would select if you dotted off the actual expression passed to Assert.That.

I’m not thinking of further benefits that a lambda would give us beyond what I’m proposing above, but we can look at them if they come to light.

In summary, I’m proposing that:

  • We use typeof(T) in PropertyConstraint.ApplyTo<T> instead of actual.GetType() to do the property search.
  • If no property was found, we fall back to doing the same search, this time on actual.GetType(), to preserve past behavior.
  • We do not loop over actual.GetType().GetInterfaces() and search for properties on each interface.

With the question of switching them around and searching in actual.GetType() first and making typeof(T) the fallback.

Either way, this would fit the good first issue label.

@nunit/framework-team what do you think?

@jnm2 Yes… I think the reason it doesn’t work has been discussed way back. But what should users expect to work and what should they not expect?

I agree with you that the framework should support some subset of the possible things users might want to use. Since they are passing in a string, I feel it should work wherever c# className.propName would work, but not elsewhere. By longer term, I was thinking of the idea of replacing the string by a lambda, which has been floating around for a while - and also not followed up AFAIK.