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
- nunit#1394 add tests and initial implementation for looking for property defined in interface — committed to nemesissoft/nunit by MichalBrylka 4 years ago
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 😃
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 theactualexpression 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
actualexpression is the interface type. Same as how C# will not find an explicit implementation unless you first cast to the interface type. SoAssert.That<Array>should not find aCountproperty, butAssert.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), whereTis the generic type parameter ofAssert.That<T>. Usually the type argument is implicit when callingAssert.That. In the initial examples,Twould beIFoobecause the initial example is implicitly callingAssert.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 onactual.GetType(), it would break tests like this:I do not like the idea of being able to find explicitly-implemented properties when
Assert.That<object>orAssert.That<ImplementingType>is called. I think that you should implicitly or explicitly useAssert.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 overactual.GetType()properties. The most backwards-compatible answer would be to prefer what we have done in the past and addtypeof(T)as a fallback if no property was found. It seems unlikely for the difference to matter in practice though. I lean towards preferringtypeof(T)overactual.GetType()because that’s the property the compiler would select if you dotted off theactualexpression passed toAssert.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:
typeof(T)inPropertyConstraint.ApplyTo<T>instead ofactual.GetType()to do the property search.actual.GetType(), to preserve past behavior.actual.GetType().GetInterfaces()and search for properties on each interface.With the question of switching them around and searching in
actual.GetType()first and makingtypeof(T)the fallback.Either way, this would fit the
good first issuelabel.@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.propNamewould 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.