Hilt: allow custom injection support for tests.

[inazaruk]

One of the key decisions with Hilt is bytecode rewriting. It helps simplify the developer experience, but makes things more complicated for testing. As a result Hilt provides additional testing framework that helps mitigate these concerns and allows for great flexibility when it comes to mocking and replacing dependencies for testing.

Still, hilt has non-trivial compilation costs. And as the codebase growth, we've observed that the cost for test complication growth even more so than for production code. As a result there is an exploration to avoid using hilt for simpler cases where the value of DI graph in tests is very small, but the additional costs to compile are great.

This diff introduces a few small touches to Hilt codegen to allow for a runtime test DI (like a simpler version of Guice) to overtake the injection.

Specifically, this diff introduces TestInjectInterceptor class with a single empty static method injectForTesting(). The codegen for Activities, Fragments, Views, Services, and Broadcasts is adjusted to have the next code:

protected void inject() {
    if (!injected) {
      injected = true;
      if (TestInjectInterceptor.injectForTesting(this)) {
        return;
      }
      // rest of Hilt injection code.
  }

For production or tests running under Hilt the additional code does nothing. And for production this code should be eliminated by R8. But for cases where testing framework is able to intercept a call to TestInjectInterceptor.injectForTesting() (like Robolectric shadow), the injection can be overtaken in a consistent manner for all types of supported android entry points.

Additional changes to @entrypoint, @ApplicationContext and @ActivityContext are done to allow runtime retention so that runtime-based testing DI can rely on annotations when resolving dependencies dynamically.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[Chang-Eric]

Hey, sorry for the delay here. Have you seen https://dagger.dev/hilt/optional-inject? I think this gives you the tools you need to use a different injector at runtime. So you could use a Hilt application in prod, but otherwise use a different injector in tests.

[inazaruk]

@Chang-Eric thanks for pointing out@OptionalInject. I wasn't aware about it and had to look deeper into what it does. It comes really close, but unfortunately is not enough to enable runtime DI in tests when using Hilt. Let me expand on why.

Test-Specific DI

To have the same context, let's assume that the codebase is fully onboarded into Hilt. Let's also assume that Robolectric is used for unit tests, specifically Robolectric shadows are used to intercept/override behavior of Android and Hilt classes.

The expected usage of such Test-Specific DI will look something like this:

public class AlarmServiceTest extends AutoMockTest {

  @Before
  public void setUp() throws Exception {

    // The `create()` call forces a call to `Service.onCreate()` which in turn 
    // triggers `Hilt_ExampleService` injection code. 
    alarmService = Robolectric.buildService(AlarmService.class).create().get();

  }

  // Tests here.

}

Where AutoMockTest applies some custom Robolectric shadows and JUnit @Rules to prepare the DI. The primary goal of AutoMockTest is to allow testing all Hilt-injected components without using Hilt.

Test-DI requirements

For AutoMockTest to support Test-Specific DI it needs these three things:

1. Universal way to intercept hilt injections into ANY android component. Or to be more precise, there should be a single class that Robolectric Shadow can intercept, otherwise it's not scalable to introduce a per-android-component Roboelctric shadow (i.e. ExampleServiceShadow, ExampleActivityShadow, and so on).
2. A reference to the android component instance during injection (i.e. reference to this). This is needed so that Test-specific DI can actually inject values into the component under test using reflection. In other words, if Hilt injection was intercepted, test DI should have a chance to do its own injection step instead.
3. Prevent Hilt from doing any work. This is needed to avoid any Hilt-specific crashes or interferences with Test-specific DI.

Comparing @OptionalInject vs current changes:

Here is how the generated Hilt_ExampleService would look like when just @AndroidComponent is used (without @OptionalInject, and without using this diff):

protected void inject() {
    if (!injected) {
      injected = true;
      ((AlarmService_GeneratedInjector) this.generatedComponent()).injectExampleService(UnsafeCasts.<AlarmService>unsafeCast(this));
    }
  }

Here is how the generated Hilt_ExampleService code looks like with @OptionalInject:

protected void inject() {
    if (!optionalInjectParentUsesHilt(optionalInjectGetParent())) {
      return;
    }
    if (!injected) {
      injected = true;
      ((AlarmService_GeneratedInjector) this.generatedComponent()).injectExampleService(UnsafeCasts.<AlarmService>unsafeCast(this));
    }
  }

And here is how the generated code looks like when this change is applied:

protected void inject() {
    if (!injected) {
      if (TestInjectInterceptor.injectForTesting(this)) {
        return;
      }

      injected = true;
      ((AlarmService_GeneratedInjector) this.generatedComponent()).injectExampleService(UnsafeCasts.<AlarmService>unsafeCast(this));
    }
  }

When using @OptionalInject:

1. 🔴 There is no way to universally intercept hilt injection. The method optionalInjectParentUsesHilt() is Android Component specific (meaning we'd have to introduce ExampleServiceShadow, and so on for each Android Component in the codebase that needs to be tested). This is not very scalable for a larger codebase, and requires additional boilerplate code for each new unit test.
2. ✅︎ If the Robolectric shadow is done on instance method (optionalInjectParentUsesHilt()), then access to instance is possible via Robloectric infra, so that's work fine here.
3. ✅︎ Since we can override optionalInjectParentUsesHilt(), the Hilt injection can be intercepted.

When using this change:

1. ✅︎ TestInjectInterceptor.injectForTesting() is a static method, and a single Robolectric shadow can be used to intercept any injections with hilt into Android Components. Making it scalable and almost no boilerplate required per class tested.
2. ✅︎ The injectForTesting() is called with a this reference, allowing the Test-Specific DI inject values at the time of onCreate() being called.
3. ✅︎ If injectForTesting() returns true, the Hilt injection is intercepted an no code from Hilt/Dagger is ever invoked beyond that point.

[inazaruk]

Also, if there is a concern about having TestInjectInterceptor in all generated code, I can update the change request to only generate TestInjectInterceptor statements based on a compiler option. To only enable for the codebases that choose to use this.

[Chang-Eric]

Hey, sorry the delay here, and thanks for the detailed explanation.

To be fairly upfront, I think it is unlikely we'll accept this change. The scope of Hilt is to enable injection through Dagger and not necessarily through other means like via runtime injection. The goal with OptionalInject is so that it is possible to make Hilt not get in the way of things, but we probably wouldn't go so far as adding a hook to replace Hilt injection since we already have support for testing in a different way. There's a page on our testing philosophy here https://dagger.dev/hilt/testing-philosophy

If you were to do this today in a non-Hilt codebase, you'd need to have each Activity/Fragment have a method that injects itself and in that injection method you could then access your TestInjectInterceptor. I think that part is still going to be necessary as we don't want to define or own this more generic injection hook, but with OptionalInject you should be able to still add Hilt to those classes while maintaining the previous injection method.

[inazaruk]

Yes, I am aware about the Hilt testing philosophy and existing approach to testing. And unfortunately it doesn't scale well in a large codebase and has significant impact on compile time. The suggestion to do this on per-component (Activity/Fragment/Service/Receiver) would probably work, but it forces production code to use @OptionalInject and forces product developers to do per-component infra setup (something they wouldn't even understand the meaning of unless they have fairly good understanding of Hilt)- which also doesn't scale well in a large codebase.

I understand the hesitance of allowing the test hook, but my hope was that if we hide this behind the flag it would be reasonable. This is not an expansive change, and it touches only a few places in Hilt and has no material impact on Hilt operation, principles and unlikely to prevent future changes in the codebase.

[Chang-Eric]

I'm sorry, I don't think I really have a satisfying answer for you.

I think our current testing approach doesn't have to have the scaling problems you mention, though we have sort of not elaborated too much on that because our experience is that many Gradle users don't try to limit dependencies too much. But Hilt is designed such that if you are able to manage what your test depends on, you can easily cut out different @AndroidEntryPoints and @Module classes that aren't used in the test and everything works nicely so that you can have small tests with small Dagger components that should alleviate your build concerns. Actually, that is my preferred way to manage replacing modules which is not using @TestInstallIn, but just not depending on the unused production module to begin with in the test if you aren't using it. This is easier done in Bazel though which is where a lot of our experience comes from, and while it is possible in Gradle, it just isn't as typical as far as I know, which is why we haven't really talked too much about this approach. It is something you might consider though.

Unfortunately though, while this test hook is simple in code right now, it just represents too large of a conceptual scope increase to the library. That scope increase just generally poses long-term risks to the maintainability of the project.