google / dagger

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

Why @TestInstallIn#replaces() cannot contain Hilt generated public wrapper modules? #4111

Open azabost opened 1 year ago

azabost commented 1 year ago

Hey. I've recently started using Hilt and currently writing some Robolectric-based tests.

In the project I'm working on, we decided to make everything internal by default, including Hilt modules which often expose the details we wouldn't like to expose, e.g. the actual classes implementing certain interfaces. In fact, the Hilt guide also recommends restricting the modules' visibility:

In Dagger, modules are usually public visibility because they are referenced by other components or other modules installing them. However, in Hilt, because modules are installed just by being in the transitive dependencies, modules don’t really need to be public for the same reason (technical aside: Hilt will actually generate public wrappers to get around visibility requirements for compilation).

In fact, doing the opposite and restricting visibility of Hilt modules is a best practice because it prevents non-Hilt Dagger components from installing the modules.

I realized having internal modules doesn't stop me from using @UninstallModule because I can reference the Hilt-generated wrapper e.g.

@HiltAndroidTest
@UninstallModules(HiltWrapper_SomeInternalModule::class)
class MyTest

However, once I started adding more test classes, I realized I would like to reuse the test bindings I previously replaced by using @BindValue. First, I tried creating an abstract class with the common bindings but it quickly turned out it was not supported https://github.com/google/dagger/issues/3405. So I decided to extract all the stuff to a separate @TestInstallIn module, but then, surprisingly, it turned out I couldn't replace the Hilt-generated wrapper anymore 🤦‍♂️

Question 1: What's the reason for that limitation? Does it mean I have to make all my Hilt modules public now?

(Side note: for now, as a workaround, I decided to turn my internal interface modules into public abstract class modules with internal abstract fun methods.)

To be honest, I think I preferred the Dagger approach where I could simply create a custom @Component for the test and use it wherever and however I wanted, including non-instrumented tests, e.g.

@Singleton
@Component(
    modules = [
        TestDispatcherSelectionModule::class // I could reuse either the test modules or the production modules here
    ]
)
interface SomeViewModelTestComponent {

    @Component.Builder
    interface Builder {
        @BindsInstance
        fun setTestDispatcherModule(module: TestDispatcherSelectionModule): Builder

        fun build(): SomeViewModelTestComponent 
    }

    fun getViewModel(): SomeViewModel
}

Question 2: Is it still possible to use the plain Dagger approach in tests while using Hilt in the production code? (e.g. can I add Hilt modules to @Component's modules list?)

Chang-Eric commented 1 year ago

For your first question, the answer is actually that that is an unintended difference between @UninstallModules and @TestInstallIn, but in the opposite way than you probably are expecting. We actually intentionally limited @TestInstallIn to work with visible modules because our thinking is that if you are able to replace the module with @TestInstallIn, that means that the module is in some sense part of your API to the tests, and so users should be able to control the visibility of that. If you don't want other tests to replace it, then you can control that by controlling the visibility.

The fact that @UninstallModules gets around this is an oversight. At this point, I'm not sure it is worth a breaking change to bring these into line, but we'll have to think about it some more.

For the second question, you could use plain Dagger and install your Hilt modules in there, though maybe not something I'd recommend? It seems like it'd be more work since you'd have to construct the components (usually more than one component like Singleton and Activity) and you might have to replicate some of the bindings Hilt provides by default in there. But with that said, I don't think there's anything currently stopping you from doing that if you find it easier.