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

Type.Should().Be(Type) doesn't support open generics #954

Closed
thepinkmile opened this issue Oct 30, 2018 · 9 comments
Closed

Type.Should().Be(Type) doesn't support open generics #954

thepinkmile opened this issue Oct 30, 2018 · 9 comments

Comments

@thepinkmile
Copy link

@thepinkmile thepinkmile commented Oct 30, 2018

I would expect this:

var response = result.GetType(); // returns ICustomInterface<int>
response.Should().Be(typeof(ICustomInterface<>));

to succeed.

Instead it fails with message:

Expected type to be ICustomInterface1, but found ICustomInterface1[[System.Int32, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].

I believe this is related to the change made for: #458

P.S. Workaround for now:

result.GetType().GetGenericTypeDefinition()
        .Should().Be(typeof(ICustomInterface<>));

I am also using this in a few "And" constraints which forces me to do:

 result.Should().NotBeNull()
    .And.Subject.GetGenericTypeDefinition().Should().Be(typeof(ICustomInterface<>));

Which is ugly and stops me being able to add more assertions as the Subject is now the GenericTypeDefinition and not the actual Type I am validating.

@jnyrup

This comment has been minimized.

Copy link
Collaborator

@jnyrup jnyrup commented Oct 30, 2018

Thanks for raising this issue.
First of all to assert on the type of an object you should probably use one of

response.Should().BeAssignableTo<TType>()
response.Should().BeAssignableTo(typeof(TType))

to test that response is assignable to TType.

The changes related to #458 added support for open generic when using BeOfType().

Here are my findings:

First let's see how Fluent assertions behaves for non-generic types to establish a baseline.

interface IInterface { }

class Base : IInterface { }

class Derived : Base { }

Derived derived = new Derived();
derived.Should().BeOfType<Derived>(); // success
derived.Should().BeAssignableTo<Base>(); // success
derived.Should().BeAssignableTo<IInterface>(); // success
derived.Should().BeOfType<Base>(); // fail
derived.Should().BeOfType<IInterface>(); // fail

Base baseClass = new Derived();
baseClass.Should().BeOfType<Derived>(); // success
baseClass.Should().BeAssignableTo<Base>(); // success
baseClass.Should().BeAssignableTo<IInterface>(); // success
baseClass.Should().BeOfType<Base>(); // fail
baseClass.Should().BeOfType<IInterface>(); // fail

IInterface interfaced = new Derived();
interfaced.Should().BeOfType<Derived>(); // success
interfaced.Should().BeAssignableTo<Base>(); // success
interfaced.Should().BeAssignableTo<IInterface>(); // success
interfaced.Should().BeOfType<Base>(); // fail
interfaced.Should().BeOfType<IInterface>(); // fail

And the results for doing the same assertions on generic types, both with open and closed generic types.

interface IInterface<T> { }

class Base<T> : IInterface<T> { }

class Derived<T> : Base<T> { }

Derived<int> derived2 = new Derived<int>();
derived2.Should().BeOfType<Derived<int>>(); // success
derived2.Should().BeAssignableTo<Base<int>>(); // success
derived2.Should().BeAssignableTo<IInterface<int>>(); // success
derived2.Should().BeOfType<Base<int>>(); // fail
derived2.Should().BeOfType<IInterface<int>>(); // fail

derived2.Should().BeOfType(typeof(Derived<>)); // success
derived2.Should().BeAssignableTo(typeof(Base<>)); // <-------------- now fails
derived2.Should().BeAssignableTo(typeof(IInterface<>)); // <-------- now fails
derived2.Should().BeOfType(typeof(Base<>)); // fail
derived2.Should().BeOfType(typeof(IInterface<>)); // fail

Base<int> baseClass2 = new Derived<int>();
baseClass2.Should().BeOfType<Derived<int>>(); // success
baseClass2.Should().BeAssignableTo<Base<int>>(); // success
baseClass2.Should().BeAssignableTo<IInterface<int>>(); // success
baseClass2.Should().BeOfType<Base<int>>(); // fail
baseClass2.Should().BeOfType<IInterface<int>>(); // fail

baseClass2.Should().BeOfType(typeof(Derived<>)); // success
baseClass2.Should().BeAssignableTo(typeof(Base<>)); // <-------------- now fails
baseClass2.Should().BeAssignableTo(typeof(IInterface<>)); // <-------- now fails
baseClass2.Should().BeOfType(typeof(Base<>)); // fail
baseClass2.Should().BeOfType(typeof(IInterface<>)); // fail

IInterface<int> interfaced2 = new Derived<int>();
interfaced2.Should().BeOfType<Derived<int>>(); // success
interfaced2.Should().BeAssignableTo<Base<int>>(); // success
interfaced2.Should().BeAssignableTo<IInterface<int>>(); // success
interfaced2.Should().BeOfType<Base<int>>(); // fail
interfaced2.Should().BeOfType<IInterface<int>>(); // fail

interfaced2.Should().BeOfType(typeof(Derived<>)); // success
interfaced2.Should().BeAssignableTo(typeof(Base<>)); // <-------------- now fails
interfaced2.Should().BeAssignableTo(typeof(IInterface<>)); // <-------- now fails
interfaced2.Should().BeOfType(typeof(Base<>)); // fail
interfaced2.Should().BeOfType(typeof(IInterface<>)); // fail

To summarize:

  • BeOfType(typeof(GenericType<>)) works as expected
  • BeAssignableTo(typeof(GenericType<>)) does not work as expected.

If agree that BeAssignableTo should have the same behavior as BeOfType when asserting on open generic types.

Whoever takes up this task, should assure consistency in the behavior between:

  • BeOfType(Type)
  • NotBeOfType(Type)
  • BeAssignableTo(Type)
  • NotBeAssignableTo(Type)

Currently only BeOfType(Type) has support for open generic types.

@thepinkmile

This comment has been minimized.

Copy link
Author

@thepinkmile thepinkmile commented Oct 30, 2018

Thanks for the quick response.

I did originally try the BeAssignableTo(typeof(...)) methods which failed. Then did an issue search and saw #458 which probably confused me into using Be(...).

I am assuming the changes for this would be similar to the referenced issue.
If I have time I might take a look at adding the required changes if no one else gets to it first and create a Pull Request for it.

@izzycoding

This comment has been minimized.

Copy link

@izzycoding izzycoding commented Oct 30, 2018

Just looking at my test code and it appears (at least to me) that I am using the correct method.
Although my example does a .GetType(), I am actually testing properties of a ServiceDescriptor.ImplementationType which is a Type object and not an actual object.

BTW: I am same person as The Pink Mile, just posting this from my mobile on the train home.

@izzycoding

This comment has been minimized.

Copy link

@izzycoding izzycoding commented Oct 30, 2018

An example of above (requires Microsoft.Extensions.DependencyInjection.Abstractions):

var sd = ServiceDescriptor.Singleton(typeof(IInterface<>), typeof(MyInterfaceImpl<>));
sd.ServiceType.Should().Be(typeof(IInterface<>));
sd.ImplementationType.Should().Be(typeof(MyInterfaceImpl<>)).And.BeAssignableTo(typeof(IInterface<>));

As i’m On train home I cannot compile and test this, but based on my observations previously, I assume this would fail.

@mdonoughe

This comment has been minimized.

Copy link
Contributor

@mdonoughe mdonoughe commented Oct 31, 2018

This sounds straightforward when the subject type is a constructed generic type of the test generic type definition.

However, if the subject type is a different type which just implements or derives from the test generic type the problem it gets more complicated.

@mdonoughe

This comment has been minimized.

Copy link
Contributor

@mdonoughe mdonoughe commented Oct 31, 2018

Should BeDerivedFrom also support open generics?

mdonoughe added a commit to mdonoughe/fluentassertions that referenced this issue Oct 31, 2018
@dennisdoomen

This comment has been minimized.

Copy link
Member

@dennisdoomen dennisdoomen commented Oct 31, 2018

Should BeDerivedFrom also support open generics?

I think so. If you derive from PresentationModel<T>, it should still work.

@mdonoughe

This comment has been minimized.

Copy link
Contributor

@mdonoughe mdonoughe commented Oct 31, 2018

What about typeof(PresentationModel<T>).Should().BeDerivedFrom(typeof(PresentationModel<>))?

(compare to typeof(PresentationModel<T>).Should().BeDerivedFrom(typeof(PresentationModel<T>)))

dennisdoomen added a commit that referenced this issue Nov 8, 2018
* Consider types assignable to open generics (#954)

* code clean up

* also consider open generics as base types

* code clean up

* make BeOfType NotBeOfType consistent

* make BeAssignableTo same for objects

* add more tests
@jnyrup

This comment has been minimized.

Copy link
Collaborator

@jnyrup jnyrup commented Nov 9, 2018

Support for open generic was contributed in #955 and released with Fluent Assertions 5.5.0.

@jnyrup jnyrup closed this Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.