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

Hilt: Supporting module uninstallation across all tests #1923

Closed
AllanWang opened this issue Jun 16, 2020 · 7 comments
Closed

Hilt: Supporting module uninstallation across all tests #1923

AllanWang opened this issue Jun 16, 2020 · 7 comments

Comments

@AllanWang
Copy link

From https://developer.android.com/training/dependency-injection/hilt-testing:

This only replaces the binding for a single test class. If you want to replace the binding for all test classes, put the test binding in the test module for Robolectric tests, or in the androidTest module for instrumented tests.

This is in reference to providing modules within a test class vs at the top level. Moving it at the top level works, but it seems like every test is still required to annotate production modules to be uninstalled. The annotation is ignored if we try to use it in an abstract super class for all tests.

What is the approach here? Should we try to also add a hilt rule in the super class? Is there a workaround to avoid uninstalling the same module across all tests?

@bcorso
Copy link

bcorso commented Jun 16, 2020

For now, the best approach is to use @Uninstall on every test. However, we're aware of this issue and looking into ways to improve the API for this use case.

Also, see #1896

@simonsickle
Copy link

An interesting thought I wanted to provide in case it had not been considered:

Since the UninstallModules and HiltAndroidTest annotations are both a part of unit testing only, could we solve this issue by adding the Inherited annotation (and would this work the way I think it does)? At that point a developer would create a base test class with some environment setup, like this

@UninstallModules(ApiConfigurationModule::class)
@HiltAndroidTest
open class BaseHiltTest {
    @get:Rule
    val hiltRule by lazy { HiltAndroidRule(this) } // lazy to prevent leaking this in non-final class

    // If the developer needs any special setup config
    open fun setup() {}

    @Before
    fun init() {
        hiltRule.inject()
        setup() // this pattern allows a dev to still call code before without remembering to call a super
    }
}

Downsides to the approach:
If the user has different modules to remove, I think they would need to remove then the UninstallModules annotation would need to be added and the developer might not realize there was a transparently added annotation removing some other number of classes which could cause confusion and wasted time.

I believe adding another method annotated with Before would replace the one in the base class - I am currently unsure of any way other than a lint check to alert the developer to what is going on in that case.

I don't think my solution is fully capable of handling those cases, but maybe it will give ideas to others.

@krgauthi
Copy link

krgauthi commented Dec 7, 2020

I've got a case where adding @Uninstall to every feature won't work. We've got our integration tests living in our feature module and the module I need to uninstall is application specific. i.e. my feature doesn't know about the module and can't uninstall it.

It would be ideal to uninstall a module via the test application class. I need to do some more investigation on how this is implemented but I need the ability to swap in and out modules on an application to application basis for shared code.

@snepalnetflix
Copy link

+1, this would really be helpful for my codebase.

@bcorso
Copy link

bcorso commented Dec 7, 2020

Hi @krgauthi,

the module I need to uninstall is application specific. i.e. my feature doesn't know about the module and can't uninstall it

Can you clarify what you mean by "doesn't know about the module"? If the feature doesn't depend on the application module then it shouldn't be installed, so there would be no need to uninstall it.

It would be ideal to uninstall a module via the test application class.

Can you describe where your test vs test application live? Are you talking about a case where the test application is in a separate Gradle module from the test?

@krgauthi
Copy link

krgauthi commented Dec 7, 2020

@bcorso This could be a lengthy explanation, I might need to make an example.

Can you describe where your test vs test application live? Are you talking about a case where the test application is in a separate Gradle module from the test?

I've got two Gradle modules one for each app, I've got a third gradle module with a shared feature from both apps that's a dependency for each app. I've currently got my Espresso tests in that feature gradle module. With Dagger I had two AppComponents for each app (4 total), one for normal builds the other for integration tests. The difference between the components was just one dagger module one had real implementations the other had stubbed implementations for the integration tests.

My issue with adding @UninstallModules to the test in the feature gradle module is that I'd be referring to a dagger module in one / both of the app gradle modules - which I don't want the feature to have a dependency on. In a multi-module app a lot of the implementation decisions happen at that application layer, dagger could accommodate and I don't think Hilt is currently - I'm still investigating though.

I think that this is a similar issue

Chang-Eric referenced this issue Jan 14, 2021
This feature allows users to replace @Installin modules with a @TestInstallIn module. For example, you can replace FooModule with FakeFooModule with the following:

```
  @module
  @TestInstallIn(
      components = SingletonComponent.class,
      replaces = FooModule.class)
  interface FakeFooModule {
    ...
  }
```

Note that @TestInstallIn only replaces @Installin when using @HiltAndroidTest, it does not apply in @HiltAndroidApp.

RELNOTES=Adds @TestInstallIn feature to Hilt
PiperOrigin-RevId: 351213437
@Chang-Eric
Copy link
Member

Closing this as the above commit should fix this issue.

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

No branches or pull requests

6 participants