google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.41k stars 2.01k forks source link

Unit test sample #186

Open mheras opened 9 years ago

mheras commented 9 years ago

We need a sample from you guys, showing us how to use Dagger 2 in unit tests (Robolectric + Dagger 2 + Mockito). I've seen a lot of workarounds (like making use of the Android variants), but none of them seem clean to me.

For example, this is another sample, and I submitted an issue to explain why I don't like it.

I cannot believe Dagger 2 was designed without considering easy & clean testing as one of its main goals.

Thank you in advance. Keep it up!

mheras commented 9 years ago

Or moreover, here is another issue with a lot of workarounds, but all of them with a little bit of bad smell...

mheras commented 9 years ago

Or here, this dev is using a boolean flag to either use the actual provisions or mocked ones. Another workaround...

Production code shouldn't be modified so that the unit tests are possible. This couples the production code with the testing code, something we should always avoid.

Testing should be a key for Dagger 2. I hope you consider improving this aspect; I guess I'll have to wait until migrating to Dagger 2.

daverix commented 9 years ago

Why bother using Dagger in unit tests? You only need to mock your dependencies. When testing Android activities you have already tested your logic in separate classes and needs integration tests to verify everything is wired together? Do you need to change Dagger modules in that case? Can't you just mock your AppComponent that you have an instance of in the Application class?

The app component can be provided through an interface from getApplication(). When creating the RobolectricTest you can just create a TestApplication which implements the interface and is set in the Config annotation. That way you can test the app works as a whole with database logic etc mocked away in your app component. (To lower the execution time of the test) On May 20, 2015 7:12 AM, "Martin Heras" notifications@github.com wrote:

Or here http://engineering.circle.com/instrumentation-testing-with-dagger-mockito-and-espresso/, this dev is using a boolean flag to either use the actual provisions or mocked ones. Another workaround...

Testing should be a key for Dagger 2. I hope you consider improving this aspect; I guess I'll have to wait until migrating to Dagger 2.

— Reply to this email directly or view it on GitHub https://github.com/google/dagger/issues/186#issuecomment-103761643.

mheras commented 9 years ago

@daverix The thing is that I don't like to add a public method such as setComponent(Object component) in my MainApplication. I would be adding that method just for testing purposes, which I think it's bad.

daverix commented 9 years ago

Here's an example what I mean...

@Component(modules=MyActivityModule.class)
public interface MyActivityComponent {
    void inject(MyActivity activity);
}

public interface MyActivityComponentProvider {
    MyActivityComponent getActivityComponent();
}

public class ProductionApp extends Application implements MyActivityComponentProvider {
    @Override
    public MyActivityComponent getActivityComponent() {
        return DaggerActivityComponent.builder().myActivityModule(new MyActivityModule()).build();
    }
}

public class FakeApp extends Application implements MyActivityComponentProvider {
    @Override
    public MyActivityComponent getActivityComponent() {
        return mock(MyActivityComponent.class);
    }
}

@RunWith(RobolectricTestRunner.class)
@Config(application = FakeApp.class)
public class MyActivityTest {
    ActivityController<MyActivity> myActivityController;
    MyActivityComponent myActivityComponent;

    @Before
    public void setUp() {
        myActivityComponent = mock(MyActivityComponent.class);
        MyActivityComponentProvider provider = (MyActivityComponentProvider) Robolectric.application;
        when(provider.getMyActivityComponent()).thenReturn(myActivityComponent);
        myActivityController = Robolectric.setupActivity(MyActivity.class);
    }

    @Test
    public void shouldTestSomethingInActivityWithMockedComponent() {
        doAnswer(new Answer() {
            @Override
            public Object answer(InvocationOnMock invocation) {
                MyActivity activity = (MyActivity) invocation.getArguments()[0]:
                activity.dependency = mock(MyDependency.class); //inject your dependencies without dagger...
                return null;
           }
        }).when(myActivityComponent).inject((MyActivity) anyObject());

        ...
    }
}

public class MyActivity extends Activity {
    @Inject MyDependency myDependency;

    @Override
    public void onCreate(Bundle bundle) {
        super.onCreate(bundle);

        (((MyActivityComponentProvider) getApplication).getMyActivityComponent().inject(this);
    }
}

I haven't run this, but something like that?

patrickcousins commented 9 years ago

@daverix That's a really cool mockito-y solution! Although it does seem like a fair amount of boilerplate :. This kinda stuff was 1 line in Dagger 1 if I remember correctly.

I agree with @mheras -- a good sample is needed. However I think the generated component helpers don't work in the test scope right now which seems odd to me given that one of the (typical) selling points of DI is to make testing easier. See: https://github.com/google/dagger/issues/110

krzysztof-jelski commented 9 years ago

@daverix Your aproach looks feasible, but it's still a lot of boilerplate to get work done. In all the workaround approaches I've seen so far (yours being one of them), I see the main problem in the fact that in unit tests we must override components. It makes it very hard to selectively override dependencies.

What I'm used to when writing tests in Spring in backend Java is to have the whole object graph created as in the real application with exception of a couple dependencies I want to mock in my tests. In Spring you can do it easily with profiles.

But when using Dagger, I can see that the only option is to override a whole component. If I want to use real dependencies in the overriden component, I'll be repeating the same code as in production. What I have expected when learning Dagger was an easy way to override either a single dependency, or a module.

One option I see is to declare a "main" component, override in tests and make it depend on production components. Only the "main" component would have mocked dependencies. But in this approach I would have to structure my components so that I can make testing possible, not according to layers/responsibilities/domain concepts.

You've also asked why we need to use Dagger in tests. That's because some things are easiest to test at Activity level with Robolectric (proper initialization of layouts, list adapters...). And obviously in this case we don't call the activity's constructor.

vaughandroid commented 9 years ago

It turns out you can effectively extend Modules by using partial mocks. As in, you create a partial mock of the Module you want to override, then mock out the provider methods which you want to override. I've created a gist here to show an example: https://gist.github.com/vaughandroid/2ac4aeb28d3e6b008fa4

vaughandroid commented 9 years ago

So, it turns out you can also extend Modules for testing by simply extending the Module class and overriding the provider methods you want to change. Dagger 2 will fail the code generation if you include the @Provides annotation though.

This, and the overly-elaborate partial mock solution, are useful workarounds but there may be pitfalls. We still really need some official support for testing (which ideally doesn't involve as much boilerplate as the current options).

ENikS commented 8 years ago

Any progress on this?

Perhaps concept of rebuilding of the graph with new modules could be introduced to the API?

testActivityComponent = myActivityComponent.Rebuilder()
                                           .myActivityModule(new TestActivityModule())
                                           .build();

The good part about this approach is that it could still be done at compile time and proper code could be generated.

ppiech commented 8 years ago

I recently started injecting unit test class using dagger like so:

@RunWith(RobolectricTestRunner.class)
public class MyFeatureTests {

    @Module
    public static class TestModule {
        @ApplicationScope @Provides
        public Dependency1 providesDependency1() {
            return mock(Dependency1.class);
        }
        ...
    }

    @ApplicationScope @Component(modules = {TestModule.class})
    public interface TestComponent {
        void inject(MyFeatureTests test);
    }

    @Inject
    Dependency1 mDependency1;
    ...

    @Before
    public void setUp() throws Exception {
        DaggerMyFeatureTestsTests_TestComponent.builder().build().inject(this);
    }

    @Test 
    ...

Has anyone else tried this approach? I don't like the amount of boilerplate code in declaring making the TestModule, but I think that could be addressed by creating a new type of module which automatically provides mocks.

The main advantage over calling the injected constructor directly is that it validates any qualified bindings.

netdpb commented 8 years ago

We are working on documentation for testing with Dagger, and documentation of best practices for building Android apps in Dagger, including for testing.

ppiech commented 8 years ago

Great, I'll look forward to it

ENikS commented 8 years ago

Could you at least give cursory overview?

netdpb commented 8 years ago

Sure, here's my TL;DR of our recommendation:

We haven't written up the specifics of this pattern for Android yet, but the general principles should still apply.

ppiech commented 8 years ago

That lets you create a separate testing component for integration/functional tests, which extends the production component but installs different modules. Specifically, it installs the same B modules, but the fake A' modules. This would be easier if components supported inheriting module lists from super-classes.

netdpb commented 8 years ago

I think there are ways to arrange your modules such that you don't really need that inheritance.

If the bindings for a feature are split into the changeable-for-testing ones (FooModule) and the unchanging ones (FooAlwaysModule), with FakeFooModule substituting for FooModule in the test component, then if both FooModule and FakeFooModule include FooAlwaysModule, then the testing component's explicitly listed modules all differ:

@Module(includes = FooAlwaysModule.class) class FooModule { /* … */ }
@Module(includes = FooAlwaysModule.class) class FakeFooModule { /* … */ }

@Component(modules = FooModule.class)
interface ProductionComponent { /* … */ }

@Component(modules = FakeFooModule.class)
interface TestingComponent extends ProductionComponent { /* … */ }

And an annotation that lets you inherit but substitute modules may end up with a fairly complex API.

ppiech commented 8 years ago

True, Module's includes field is a form of inheritance, though without the ability to override. What I find difficult is that the only thing connecting the component interfaces' hierarchy and modules hierarchy is naming convention. But, I agree that introducing a modules hierarchy through java inheritance at this point would probably do more harm than good.

patrickcousins commented 8 years ago

Thank you guys for accepting this issue, and communicating more with us. I know at least I have been watching this issue for a long time waiting for more official explanation of how Dagger 2 is supposed to work with testing. There are quite a few hacks going around but none seem ideal.

@netdpb Nevertheless, I find this recommendation really surprising:

Don't use Dagger for unit tests; just instantiate the class being tested and pass its dependencies manually.

Given this from the 4th paragraph fourth the Dagger home page:

By building on standard javax.inject annotations (JSR 330), each class is easy to test. You don't need a bunch of boilerplate just to swap the RpcCreditCardService out for a FakeCreditCardService.

Isn't this supposed to be one of the main benefits of dependency injection?

How is it that the recommendation for a dependency Injection library is not to use it for dependency injection?

I understand you are referring specifically to unit testing, but a lot of unit tests are not perfectly isolated, and (IMHO) I am not sure how Dagger 2's inflexibility on this issue helps things.

I guess it would be nice to understand what the thinking is here. It seems like Dagger 2 is more hostile to testing where Dagger 1 was very helpful. But I may not be understanding the whole picture. Thanks again for your time and communicating with us.

JakeWharton commented 8 years ago

How is it that the recommendation for a dependency Injection library is not to use it for dependency injection?

Dependency injection does not need a library to be used. The recommendation was not to use the library but to just use the pattern.

ENikS commented 8 years ago

If done in accordance with best practices for Android Studio and utilizing 'variant' based implementation this could be achieved pretty cleanly:

1 Create interface declaring injectors and getters:

interface IComponent { 
    SomeType getSomeType();
    /* … */
    inject(SomeType obj);
 }

2 Declare production component in prod variant

@Component(modules = { FooModule.class, XyzModule.class} )
interface XyzComponent extends IComponent  { }

3 Override production component in test variant

@Component(modules = { TestFooModule.class, XyzModule.class} )
interface XyzComponent extends IComponent  { }

Note: Prod and Test components do not have any members. It is simply connection point for modules. Main problem in testing of components is a tight coupling of Component and modules associated with it. By decoupling declaration into separate interface we also decoupling behavior from component implementation.

netdpb commented 8 years ago

@patrickcousins Yes, @JakeWharton has it right. One benefit to using DI is that your classes become easier to test. That doesn't mean you use Dagger when doing those tests; it means your class allows its instantiator to hand it its dependencies, whether that instantiator is Dagger (or another DI framework) or a unit test setUp() method.

That advice is only really relevant for small unit tests, where you're testing one class in relative isolation, and not part of a larger cluster or application.

You also need bigger, functional-style tests that exercise the whole application, with some test duplicates. For those, you'd use Dagger in a test configuration.

It may be that most tests of Android applications fall into the functional-style bucket.

netdpb commented 8 years ago

@ENikS That makes sense; it's consistent with what I suggested above.

One additional tweak: it may be useful for the testing component type to add some extra provision methods so that the test harness can get directly at some of the fakes. For instance:

@Component(modules = {FooModule.class, OtherModules.class})
interface ProductionComponent {
  SomeRootType someRootType();
}

@Component(modules = {FakeFooModule.class, OtherModules.class}) {
  FakeFoo fakeFoo();
}

Your tests may want to call methods on FakeFoo directly in order to manipulate or inspect the test application in ways inappropriate for production.

patrickcousins commented 8 years ago

Thank you @JakeWharton and @netdpb . I guess I was thinking of members injection but just realized that injected members would be package private anyways so could be set directly by a test class in the same package.

netdpb commented 8 years ago

@patrickcousins Yes!

ppiech commented 8 years ago

In support of unit testing, has anyone considered having a dagger module that automatically injects mocks (an equivalent of MocktoAnnotations.initMocks())? Something like:

@Module 
class MockModule {
@Provides(type=FACTORY)
<V> V provides(Class<V> clazz) {
    return Mockito.mock(clazz);
} 

It would mean that dagger would need to deal with multiple bindings which may be complicated. Also it would quite dangerous if someone tried using it in production.

ppiech commented 8 years ago

BTW, the reason I'm trying to use dagger to inject in unit tests is because I had a regression related to a refactoring that changed a qualifier on an injected type. The bug was subtle and not caught till much later in testing.

In general, there is a lot of logic embedded in the dagger graph that is not covered by unit tests and possibly not covered by functional tests if those functional tests use a different graph.

dalewking commented 8 years ago

@ENikS Test variants are definitely not an Android best practice. I should not have to create a variant for testing.

This basically boils down to what I keep saying that DI is about being able to choose at runtime what gets injected and what is broken in Dagger 2 (see https://github.com/google/dagger/issues/213). Compiling differently for test is certainly NOT the answer.

ENikS commented 8 years ago

@dalewking I could not agree with either of your statements.

You don't need to create different variants to test, although it makes code cleaner if you do. If you follow suggestion in my early post, instead of variants you could create two different components:

@Component(modules = { FooModule.class, XyzModule.class} )
interface ProdComponent extends IComponent  { }

@Component(modules = { TestFooModule.class, XyzModule.class} )
interface TestComponent extends IComponent  { 
   ... extra test members ...
}

and access these by IComponen. By doing so you override required modules, keep the same code base and have completely decoupled implementation. After all this is what interfaces are for...

As for resolving dependencies at runtime you are not correct : dependencies are always resolved either at compile time (registration methods in case of Unity container or attributes in MEF) or configuration time (Spring, etc.). At runtime you just simply resolve and use whatever is configured for the type.

Dagger 2 moves this determination phase to compile time and uses static linking to increase performance.

As for the #213 issue, ideally you should not call inject methods in your code. If you do, your design is wrong. You need to get away from reflection centric approach and think about device with just 64K of memory...

dalewking commented 8 years ago

@ENikS I understood your example, I disagree with the notion that separate variants is a best practice and anything other than a hack to work around the problems with Dagger 2.

When I say resolved at runtime, I mean the choice of component is chosen should be done at runtime. I should be able to compile the class to be injected once. That same class instance can be run in the production code where a production component is used or run in a unit test where a testing component is used.

Creating an interface is a step in the right direction, but still does not resolve all issues. You will end up with boilerplate in every class that needs injection.

In Android how are you proposing that you avoid calling inject methods?

And no I am not talking about a reflection approach. You have to be clear what you mean by reflection. Calling getClass() and using the result to look up what method injector to use. That is not the same as reflection.

ENikS commented 8 years ago

@dalewking I have a feeling you are trying to drive a nail with a microscope with insistence on runtime resolution. There are proper tools for everything. If you want to build graphs at runtime you should use Dagger 1 of Guice or Spring.

As I understand Dagger 2 was specifically designed to make all of its decisions at design time and avoid runtime resolution all together. But if you need to resolve type at compile time, the only way to do it if you specify the type in your code. So, here comes the "limitation"...

@netdpb I think as a compromise annotation processor could generate and automatically include into component implementation method with Object type parameter like this:

interface ComponentBase  {
   Inject(Object obj);
 }

The implementation of the method would have an injector for each known injectable type:

public class DaggerProdComponent implements ProdComponent, ComponentBase {
 ...
  Inject(Object obj) {

      switch(obj.getClass().getName()) { 

          case: "type1": // could be hashcode, index, (type1.class).getName() or whatever...
              inject((type1)obj);
              break;

          case: "type2":
              inject((type2)obj);

          // All known injectable types
          ...
         default:
            throw new Exception("Unknown type");
      }
  }
}
 ...

There will be a small performance impact if you use it but it could be avoided if type injectors called directly. So, you'd have best of two worlds.

netdpb commented 8 years ago

See my suggestion at https://github.com/google/dagger/issues/213#issuecomment-164022841 for a way to implement this outside of Dagger itself.

kvernon commented 7 years ago

I would love an example over testing a daggerservice or daggerintentservice with mocking.