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

Requiring 'injects' make unit-test injection cumbersome #269

Closed
diwakergupta opened this issue Jun 4, 2013 · 13 comments

Comments

@diwakergupta
Copy link

commented Jun 4, 2013

We use junit4 for tests and most of our tests require injection for some fields. With Guice, we can simply have a TestRule that injects the members:

// module is a test module that overrides production settings
Guice.createInjector(module).injectMembers(target);

Unfortunately, Dagger requires modules to declare entry points via the 'injects' clause and there doesn't seem any way to relax that. This effectively makes it impossible to use a Rule based approach like above. Instead, each test has to specify it's own module:

class MyTest {
  @Module(injects = MyTest.class, includes = TestModule.class)
  static class MyTestModule {
  }
}

This results in a lot of boilerplate code across all tests. I understand the 'injects' clause allows Dagger to flag errors up front, but there should be an easier way to inject members in unit tests.

@cgruber

This comment has been minimized.

Copy link
Collaborator

commented Jun 4, 2013

It's not just "allowing flagging of errors" - it's the basis for roots of the full graph analysis. I think there might be value in finding some testable approach.

What I would say is, firstly, consider mocking out the injector for all but end-to-end integration tests, and second, in an actual unit test you shoudn't ever be creating an injector. You should be testing your components in isolation.

But for full-scale integration tests, I can see the value of being able to assert state on arbitrary objects to more finely tune error detection.

@diwakergupta

This comment has been minimized.

Copy link
Author

commented Jun 4, 2013

While this is a perfectly valid ideological standpoint, there are plenty of
practical use-cases for using injectors in a unit test. Mocks are
incredibly useful, but are not suited everywhere. As one example, we use
@nAmed bindings to generate random port numbers for tests. What you choose
to call "unit" vs. "integration" test is often subjective -- in my case,
many unit tests depend on some other components for testing, hence the need
for injection.

@swankjesse

This comment has been minimized.

Copy link
Member

commented Jun 5, 2013

Agreed that injects is annoying. I'm planning to do something different for Dagger 2.0, when we can do full-application analysis.

@cgruber

This comment has been minimized.

Copy link
Collaborator

commented Jun 5, 2013

Greg and I are exploring some alternatives as well around entry points,
including some nice pre-linking that should remove all loading except
the first load of the entry point's adapter. I think we need to throw
together some shared docs on ideas for how to approach it, and hash out
what 2.0 might look like.

@diwakergupta

This comment has been minimized.

Copy link
Author

commented Jun 5, 2013

@swankjesse, @cgruber awesome, look forward to it! I guess it's premature but I'll still ask -- any timeline for a 2.0 release? :-)

@cgruber

This comment has been minimized.

Copy link
Collaborator

commented Jun 5, 2013

now() + N minutes. :) No idea as we don't even know what we want in it.

@jloisel

This comment has been minimized.

Copy link

commented Jun 24, 2013

From my point of view, unit tests should test components isolated from each other by using mocks, or tracking down a test failure can really become painful.

We do test objects being injected by providing instances manually, this avoids that unit tests to fail when injection is not working.

We have tests on modules checking that wiring is working fine. These are the only tests we have that use injection. This allows use to quickly figure out if our code is failing because of wiring or broken code inside an object.

Objects should be simple enough to be mocked easily. We try to have mostly single method interfaces with obvious contracts.

Maybe, there should be something dedicated to integration unit tests in Dagger allowing to relax some settings, although this is pretty dangerous as it may be misused in production code.

@diwakergupta

This comment has been minimized.

Copy link
Author

commented Jun 24, 2013

On Mon, Jun 24, 2013 at 3:06 AM, Jerome notifications@github.com wrote:

From my point of view, unit tests should test components isolated from
each other by using mocks, or tracking down a test failure can really
become painful.

This is a fine point of view but in practice, I've seen most code bases
that use injection also relying on injection for testing. Existence of
projects such as GuiceBerry (https://code.google.com/p/guiceberry/) and
Jukito (https://github.com/ArcBees/Jukito) further support my anecdotal
observations.

Anyways, just wanted to raise this so we can have a discussion. If the
Dagger community doesn't find this useful, feel free to close as won't fix.

@jloisel

This comment has been minimized.

Copy link

commented Jun 25, 2013

There are also libraries like PowerMock (http://code.google.com/p/powermock/) but that does not mean it's a good practice to use them. I understand your point of view, working with legacy code often requires to make compromise. I'm open to having something in Dagger satisfying both worlds.

@christopherperry

This comment has been minimized.

Copy link

commented Jan 24, 2014

You only run into this problem where you aren't doing proper dependency injection (passing dependencies via constructor). If you are testing your class in isolation, and passing all dependencies in your constructor then you have zero need for dagger to be involved at all in your test class.

Sometimes though, it's unavoidable because you don't have control over the creation of the class (i.e. you can't provide your own constructor to do proper DI). Activity, Fragment, Service, View, etc are all problematic in this way, so these are really the only classes where you are forced into using field injection and Dagger in your test class.

@cgruber

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2014

In these cases, however, you can provide a test module (often with
overrides=true) that narrowly defines the types you need to pull out to
examine in the test, or even better, a TestEntryPoint class that simply
injects into itself bits of graph state for the test.

@christopherperry

This comment has been minimized.

Copy link

commented Mar 13, 2014

@cgruber That's exactly what I'm doing, and I've got something that hooks into the Activity/Fragment creation process to allow me to define mocked behaviors at the right time (I'm using Robolectric.buildActivity().create().etc().etc() to start my Activities in test, which presents timing issues for defining mocked behaviors for things that run in onCreate() etc). I could share if you guys would like, I'm open to comments/suggestions.

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

commented Nov 16, 2014

injects= is dead in v2 and there's recommendations above for workarounds for v1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.