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

Use dependency injections for test. (Ninject.MockingKernel.Moq) #661

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bnordli
Copy link
Contributor

@bnordli bnordli commented Jun 22, 2015

Just a suggestion to simplify testing

@einari
Copy link
Contributor

einari commented Dec 18, 2016

I know this is super late to be discussing this; but its been a hectic 18 months :)

Anywho. Why would one need to have the IoC container involved when writing specs?
I've never had the need to have the IoC there. We write the code without thinking about IoC - but follows the DI principle.

@bnordli
Copy link
Contributor Author

bnordli commented Dec 18, 2016

Welcome back! :)

It is certainly debatable, but we have found it to greatly simplify writing specs:

  1. In the vast amount of cases, injections into test subjects are mocks. Using the IoC container for tests reduces the boilerplate for initializing these mocks. Also, you don't need to store references to these mocks, but can use the container to retrieve them. (See for instance https://github.com/dolittle/Bifrost/pull/661/files#diff-409e2a1bc119b802accd8d5e1f1da9a3 for an example of simplification.)
  2. When the test subject constructor changes (because of new or removed injections), you normally would need to go to all instantiations of this class in tests and add/remove this new dependency. When using IoC in tests, the constructor is never referenced, and the new dependency is automatically injected (as a mock).

btw, this has already been accepted into the ProCoSys fork here: ProCoSys#11

@einari
Copy link
Contributor

einari commented Dec 27, 2016

:) Thanks. Its good to be back.

Thanks for the clarification. I think I see and understand the need, but I'm still reluctant to use the IOC for this.

I think it would be better to have something that optimized for developer experience surrounding this.

Let me sleep another fourthnight on it. :) I'll keep the PR here - please don't close it in any way - good for reference and me understanding it all..

@bnordli
Copy link
Contributor Author

bnordli commented Dec 29, 2016

What else would you like to use?

It would certainly be possible to write a custom tool for this, but as the project already uses Moq for mocking, and has an implementation for Ninject, both should be familiar to developers, so it sounds reasonable to use an existing framework to do this. (To be fair, there are no direct dependencies from the test themselves to Ninject.)

The possible downsides I can see are

  1. Dependency on yet another framework (but only in tests): Discussed above.
  2. Added time for tests: I haven't made specific measurements, but the ProCoSys project has made this a standard for tests, and I haven't seen any noticeable degradation.
  3. If any dependencies are removed from the tested class, the tests that still include and test for this dependency don't break during compile time. (Without using this, the broken tests would be discovered and fixed/removed earlier.)

@bnordli
Copy link
Contributor Author

bnordli commented Jan 6, 2017

Seems like Ninject.MockingKernel does not support .NET Core (yet). So either we have to wait for it to get updated, or use another DI framework.

@einari
Copy link
Contributor

einari commented Feb 25, 2017

Marked for waiting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants