Bug: Assert.That(IEnumerable<Type>, Has.All.Property(nameof(Type.Namespace)) fails.

[andrewimcclement]

Originally posted as https://stackoverflow.com/questions/74475185.
Using NUnit v3.13.2, .NET Framework 4.8, running using ReSharper test runner. I'll try to find some time to run this with just nunit console runner and/or actually debug this myself.

I want to assert that all the types I am interested in are in the expected namespace using NUnit.

IReadOnlyCollection<Type> types = GetInterestingTypes(); // Method defined elsewhere.
Assert.That(types, Has.All.Property(nameof(Type.Namespace)).StartsWith("MyNamespace"));

This fails with ArgumentException Property Namespace not found on ....
(Note the StartsWith is irrelevant.)

However, the equivalent non-collection assert Has.Property(nameof(Type.Namespace)).StartsWith("MyNamespace") succeeds.
I assume that this is a bug, if not, let's make sure the documentation is clear about how this is supposed to work.

Minimal reproducible example:

using System;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;

using NUnit.Framework;

namespace MyNamespace
{
    internal class MyTests
    {
        [Test]
        public void TestsWithinNamespace()
        {
            var types = new[] {typeof(Foo), typeof(NestedNamespace.Bar)};

            foreach (var type in types)
            {
                // Passes
                Assert.That(type, Has.Property(nameof(Type.Namespace)).StartsWith("MyNamespace"));
            }

            // Fails
            Assert.That(types, Has.All.Property(nameof(Type.Namespace)).StartsWith("MyNamespace"));
        }
    }

    public class Foo { }

    namespace NestedNamespace
    {
        public class Bar { }
    }
}

When running this test, we get

System.ArgumentException : Property Namespace was not found on MyNamespace.Foo.
Parameter name: name
   at NUnit.Framework.Constraints.PropertyConstraint.ApplyTo[TActual](TActual actual)
   at NUnit.Framework.Constraints.AllItemsConstraint.ApplyTo[TActual](TActual actual)
   at NUnit.Framework.Assert.That[TActual](TActual actual, IResolveConstraint expression, String message, Object[] args)
   at MyNamespace.MyTests.TestsWithinNamespace()

Potentially relevant comment from Ralf on the stackoverflow question:
Debugging through NUnit i would assume a bug here. When using Property on the AllItemConstraint there is an extra handling when the thing in there is already a type. It does not create a type on the type that you would need for your code to work. Now it searches "Namespace" on Foo and not on the type object of Foo.

[stevenaw]

Thanks for the details and minimal reproduction @andrewimcclement
I can confirm this is a bug in 3.13.3 as well as the latest in main.

As you say, there's a difference in the type of TActual here:
https://github.com/nunit/nunit/blob/master/src/NUnitFramework/framework/Constraints/PropertyConstraint.cs#L40

// When run individually
actual.GetType().Name
"RuntimeType"
typeof(TActual).Name
"Type"

// When run as part of 'AllItemsConstraint'
actual.GetType().Name
"RuntimeType"
typeof(TActual).Name
"Object"

I suspect there's a boxing going on in AllItemsConstraint before it can pass the items into the BaseContraint. While I haven't tested this, it might just be a matter of changing the object to a var here when we iterate over the collection: https://github.com/nunit/nunit/blob/master/src/NUnitFramework/framework/Constraints/AllItemsConstraint.cs#L43

[manfred-brands]

@stevenaw TActual is lost in the AllItemsConstraint

AllItemsConstraint is not an IEnumerable<TActual> but just IEnumerable which returns object. Therefore TActual in the PropertyConstraint is also object.

This shouldn't be an issue as the code retrieves the real type here: https://github.com/nunit/nunit/blob/master/src/NUnitFramework/framework/Constraints/PropertyConstraint.cs#L74

That code works for all other types, except somehow for Type itself:

type.Namespace
"Tests"
type.GetProperty("Namespace")
null

Removing the "code exception" for Type and change the code from:
Type actualType = actual as Type ?? actual.GetType();
into
Type actualType = actual.GetType();

Seems to fix this. (when changing the value in the debugger).
This special treatment of Type in the code (in a slightly different form) existed according to git since 2009

I'll create a PR to see if this breaks any existing tests.

[stevenaw]

Thanks for the in-depth look and analysis @manfred-brands , appreciate it!

[manfred-brands]

The problem is distinguishing between:

1. passing in typeof(Array) and check if the Array type has a Length property or
2. passing in typeof(Array) to see if the type instance itself has a Namespace property.

There are actually two different property constraints: PropertyExistsContraint and PropertyConstraint where the latter expects an instance to allow further constraining, e.g. EqualTo

[mikkelbu]

I'm on the phone but this sounds very similar to #2436, but I've only skimmed the thread

[manfred-brands]

@mikkelbu yes that issue looks the same. Somehow that high priority issue from 2017 got forgotten. I guess there are only a few places where it surfaces.