Add new namespace assertions to TypeSelectorAssertions

[kimsey0]

Description

Currently, TypeSelectorAssertions only contains methods for asserting that types are (not) decorated with specific attributes, e.g., BeDecoratedWith<>. However, TypeSelector contains more selectors that would also be useful as assertions.

One such group of selectors filter on the namespace of types and contain the methods ThatAreInNamespace, ThatAreNotInNamespace, ThatAreUnderNamespace, and ThatAreNotUnderNamespace. Corresponding assertions could be named BeInNamespace, NotBeInNamespace, BeUnderNamespace, and NotBeUnderNamespace.

Use case

A concrete use case I have that prompted this suggestion is that the JsonSubTypes library, which is used to deserialize JSON into one of a number of subclasses based on a discriminator value, only supports deserializing into subclasses in the same namespace as the base class. Therefore, I would like to be able to write a test that asserts that all subclasses of a specific class are in the same namespace as it, à la:

AllTypes.From(baseClassAssembly).ThatDeriveFrom<BaseClass>().Should().BeInNamespace(baseClassNamespace);

Draft implementation as custom extension

To support the use case above, I have implemented the BeInNamespace assertion in our project as the following custom extension:

public static class TypeSelectorAssertionsExtensions
{
    public static AndConstraint<TypeSelectorAssertions> BeInNamespace(this TypeSelectorAssertions typeSelectorAssertions, string @namespace, string because = "", params object[] becauseArgs)
    {
        Execute.Assertion
               .BecauseOf(because, becauseArgs)
               .ForCondition(@namespace != null)
               .FailWith("Types cannot have a null namespace")
               .Then
               .Given(() => typeSelectorAssertions.Subject)
               .ForCondition(types => types.All(type => type.Namespace == @namespace))
               .FailWith("Expected types to be in namespace {0}{reason}, but found {1}.",
                   _ => @namespace, types => types.Where(type => type.Namespace != @namespace));

        return new AndConstraint<TypeSelectorAssertions>(typeSelectorAssertions);
    }
}

[jnyrup]

Looks like useful additions - I didn't think about those when I initially wrote #645
Note that it is valid for a Type to have a null namespace.

[kimsey0]

You could potentially also add DeriveFrom, NotDeriveFrom, Implement, and NotImplement to mirror the same selectors in TypeSelector, but that might be for another issue.

Ah! I assumed that types in the global namespace would have Namespace = "", but it's null. It's interesting if you can have an empty string namespace then.

[dennisdoomen]

Good suggestion indeed. We are open to a PR ;-)

[kimsey0]

I just noticed that the current ThatAreUnderNamespace filtering method in TypeSelector checks if type.Namespace?.StartsWith(@namespace) == true, which means that it would include, e.g., Countdown.LaunchCountdown under the namespace of Count.GeigerCounter. Shouldn't it instead be checking something like type.Namespace == @namespace || type.Namespace?.StartsWith($"{@namespace}.") == true?

[jnyrup]

I'm not sure whether the original intention of ThatAreUnderNamespace was to allow arbitrary string prefixes of namespace or parent namespaces, but I agree that it's unclear.

I do note that changing it to the way you specify could break the following made-up test, which validly relies on the current behavior.

namespace FOO.BAR
{
    class A {}
}

namespace FOO.BAZ
{
    class B {}
}

AllTypes.From(...)
    .ThatAreUnderNamespace("FOO.BA")
    .As<IEnumerable<Type>>()
    .Should()
    .HaveCount(2);

[kimsey0]

Yep. Changing it would be a breaking change. However, I think the filter name specifies the intention that it looks at nested namespaces, not namespace prefixes. Otherwise, it could have been named ThatHaveNamespacePrefix or ThatHaveNamespaceStartingWith. FOO.BAR isn't really under FOO.BA.

[kimsey0]

In any case, shouldn't I implement and test that the BeUnderNamespace assertion matches the ThatAreUnderNamespace filter behavior?

[jnyrup]

Indeed they should match.
Feel free to open another issue about the ambiguity.

[kimsey0]

I'll do that when these new assertions have merged.

Other inconsistencies/bugs:

• new TypeSelector(typeof(ClassInGlobalNamespace)).ThatAreUnderNamespace(null).Count() == 0 and new TypeSelector(typeof(ClassInGlobalNamespace)).ThatAreNotUnderNamespace(null).Count() == 1.
• ThatAreUnderNamespace(null) and ThatAreNotUnderNamespace(null) throw for types not in the global namespace.

Should I also just mirror that behavior in the assertions?

[jnyrup]

I'm not sure I follow you (My brain has been in complete weekend mode ever since I had brunsviger for breakfast).
Can you provide some more complete examples with the expected and actual outcome, thanks.

[kimsey0]

I completely understand what brunsviger can do to the brain. Here are some examples:

For classes:

class ClassInGlobalNamespace { }

namespace DummyNamespace {
    class ClassInDummyNamespace { }
}

And for type selectors:

var typeInGlobalNamespace = typeof(ClassInGlobalNamespace);
var typesInGlobalNamespace = new TypeSelector(typeInGlobalNamespace);

var typeInDummyNamespace = typeof(ClassInDummyNamespace);
var typesInDummyNamespace = new TypeSelector(typeInDummyNamespace);

I would expect that:

typesInGlobalNamespace.ThatAreUnderNamespace(typeInGlobalNamespace.Namespace).Count()
== typesInGlobalNamespace.ThatAreUnderNamespace(null).Count()
== 1

typesInGlobalNamespace.ThatAreNotUnderNamespace(typeInGlobalNamespace.Namespace).Count()
== typesInGlobalNamespace.ThatAreNotUnderNamespace(null).Count()
== 0

typesInDummyNamespace.ThatAreUnderNamespace(null).Count()
== 1

typesInDummyNamespace.ThatAreNotUnderNamespace(null).Count()
== 0

However, with the existing implementation:

typesInGlobalNamespace.ThatAreUnderNamespace(typeInGlobalNamespace.Namespace).Count()
== typesInGlobalNamespace.ThatAreUnderNamespace(null).Count()
== 0

typesInGlobalNamespace.ThatAreNotUnderNamespace(typeInGlobalNamespace.Namespace).Count()
== typesInGlobalNamespace.ThatAreNotUnderNamespace(null).Count()
== 1

typesInDummyNamespace.ThatAreUnderNamespace(null).Count()
// throws an ArgumentNullException

typesInDummyNamespace.ThatAreNotUnderNamespace(null).Count()
// throws an ArgumentNullException

[jnyrup]

Just to summarise the two bugs I'm seeing and agree with:

• The type ClassInGlobalNamespace is in global namespace and thus have Namespace = null, so we typesInGlobalNamespaceThatAreUnderNamespace(null) should include that type.
• Types in global namespace has Namespace = null and any type is "under" the global namespace, so typesInDummyNamespace.ThatAreUnderNamespace(null) should return all types.

The second tests aren't failing for me?

typesInGlobalNamespace.ThatAreNotUnderNamespace(typeInGlobalNamespace.Namespace).Count()
== typesInGlobalNamespace.ThatAreNotUnderNamespace(null).Count()
== 0

[kimsey0]

Oh, you're right. typesInGlobalNamespace.ThatAreNotUnderNamespace(null).Count() == 0 does hold. I must have gotten confused.

If you want to keep the concise form of the existing ThatAreUnderNamespace and ThatAreNotUnderNamespace methods, I'd suggest their filtering be changed to @namespace == null || type.Namespace?.StartsWith(@namespace) == true and @namespace != null && type.Namespace?.StartsWith(@namespace) != true.

I'll create a new issue for this.

[dennisdoomen]

I just noticed that the current ThatAreUnderNamespace filtering method in TypeSelector checks if type.Namespace?.StartsWith(@namespace) == true, which means that it would include, e.g., Countdown.LaunchCountdown under the namespace of Count.GeigerCounter. Shouldn't it instead be checking something like type.Namespace == @namespace || type.Namespace?.StartsWith($"{@namespace}.") == true?

This is most definitely a design flaw. We never intended to be able to check a namespace against a partial string

[jnyrup]

I've opened #1196 to track issue with ThatAreUnderNamespace matching partial namespaces.

[kimsey0]

Thanks for fixing the partial and null namespace issues in #1197. (I never got around to creating the issue for it. Sorry.)

Now that it's merged, I'll get #1180 up to date and ready to merge.