Fix for bug where ExtensionsForBindingSyntax.ToMock<T>() always returns null

[Sebazzz]

There was a problem where the ExtensionsForBindingSyntax.ToMock<T>() method always returns null. See the following line:

return builder as IBindingWhenInNamedWithOrOnSyntax<T>;

builder will never be of type IBindingWhenInNamedWithOrOnSyntax<T>.

I have also a method that calls IBindingWhenInNamedWithOrOnSyntax<T>.ToSingleton() for convience as this is what one usually wants in a unit test.

[kina]

+1

[Sebazzz]

Any update on this? @remogloor @danielmarbach

[fabian-we]

Please merge this fix! This is a clear bug!

[remogloor]

Fixed in https://github.com/ninject/ninject.mockingkernel/commit/49a1727151e7d7bdf2f36941a8827b242767599c

Regarding ToSingleton() and interface is a singelton mock by default. There is no need to specify them explicitly. I don't see a real need for this unless you have some good arguments.

[fabian-we]

Well, it wasn't singleton in my case. I had a binding like this: Bind().ToMock(); But I had the problem that the instance ninject injected into my tests was not the same that I did the mock setup on. With .InSingletonScope() or .ToMockSingleton() it works as expected.

[Sebazzz]

Exactly. By default, a non-singleton mock is created. If you want to mock anything you get only the mocking object but configuring it has not any use because a new one is created when its injected.

[fabian-we]

That's what I thought at first, but if you simply don't define a binding you get a singleton. So this only becomes a problem if you want to use some options from IBindingWhenInNamedWithOrOnSyntax<>.

[Sebazzz]

Isn't that a bug then?

[fabian-we]

What is a bug? That you get a singleton binding if you do no explicit binding? I think that's quite convenient.

[Sebazzz]

No, that you do not get a singleton when you use IBindingWhenInNamedWithOrOnSyntax<>. If you don't get a singleton, you cannot configure the object.

[fabian-we]

Perhaps there is a usecase where you will need this. I can't think of one, but that doesn't mean that there is none.

[Sebazzz]

I'm sorry for commenting on this, but I want to understand the reasoning behind this: why is there a use case for not being able to configure a mock?

[fabian-we]

Well, that has to be answered by remogloor. I'm only a user of this library.

[remogloor]

I don't see many situations where you have to use ToMock at all. The only situations is when you inject multiple instances of an interface into one SUT. Therefore I personnally don't care a lot if it is a singleton or not because adding the scope isn't really a big thing for those rare situations.

I see advantages/disadvantages for both singleton or transient default:

Singleton: Yes I can't think of any situation where you don't want it to be a singleton eiher. So you will configure all these bindings as singleton.

Transient: This was the behavior up to now. Changing it to singleton would be a breaking change so changing the behavior must go into a major release. Also transient is the default behavior for all other bindings except ToConstant. Therefore it is less confusing that it behaves the same way as all other bindings instead of having an own behavior.

There are ideas to make the default scope configurable in Ninject core. This might be the better way to solve this problem. That way you could make singleton the default behavior for the mocking kernel for all bindings.

[fabian-we]

I have another use case for using ToMock: I wanted to mock a class directly (without interface) and this class needed some constructor arguments that could not be supplied by ninject. So I used ToMock() and WithConstructorArgument() (and InSingletonScope of course).

But I agree that this is a breaking change! Making the default scope configurable would surely be the best solution.