Feature Request: lint rule for EarlyAccessPoint classes to prevent @Inject abuse

[kenyee]

I'm trying to add support for @HiltAndroidTest for E2E/instrumentation tests and we have a custom application class so we need to use

@CustomTestApplication(OurApp::class)
interface OurHiltTestApplication

But we also have a bunch of prefetch/init objects we need to run in app:onCreate().
Since we can't use @Inject in the OurApp class, we're forced to use EarlyEntrypoints.get() to explicitly grab this set of objects.

However, if you do this as mentioned here: #2033 (comment)
there's the potential of people grabbing new instances of these classes from the SingletonComponent because that component is different from EarlyEntryPoint's component.

Could you add a lint rule that fails @Inject of any of these classes:

  @EarlyEntryPoint
  @InstallIn(SingletonComponent.class)
  interface HiltEntryPoint { 
      Foo getFoo(); 
  }

e.g. if someone tries to @get:Inject lateinit var foo: Foo this should fail w/ a lint error. They should be required to use EarlyEntryPoints.get(Foo::class).

[bcorso]

But we also have a bunch of prefetch/init objects we need to run in app:onCreate().

Do you have an example of what you are retrieving from the EarlyEntryPoint and how you are using it?

Is it immutable?

• If it's immutable, does it really matter which component you get it from for the correctness of your test?
• If it's not immutable, is it actually safe to share these prefetched/init objects across every test/test-case since you can't really guarantee what state the mutable object is in?

Could you add a lint rule that fails @Inject of any of these classes:

The issue is more complicated than just failing for Foo. For example, if Foo depends on Bar then you are also creating Bar from the EarlyEntryPoint's component as well, so you would need a way to search for all transitive dependencies of Foo, which isn't really feasible to do without having access to Dagger's binding graph.

[kenyee]

Some of the stuff is what you'd expect to be there....e.g. startup listeners, error handling, and logging services. They're all immutable though better if there were single instances (e.g. logging should only happen in a single background thread, but if there are two instances by using @EarlyEntryPoint and @Inject, there would be two threads).

Other stuff is not...e.g. a servicemanager which is essentially a service locator for other services (this is an old app so it's partially DI and partially "reach and to a service locator and grab instances"). Good point about the transitive dependencies..the service locator would be our biggest pain point for this and I don't see a way we can use this @EarlyEntryPoints feature because we'd have duplicate objects... We'd have to rewrite our entire app startup path (where most of the init is) and try moving all initialization over to a trampoline/splash screen.

[bcorso]

You might be able to add some manual indirection to try to guarantee that users get the same instance.

For example, say that you are currently binding your logger like this:

@Module
@InstallIn(SingletonComponent.class)
interface LoggingModule {
  @Binds Logger bindLogger(LoggerImpl impl);
}

You could add a level of indirection to guarantee that your users automatically get the same logger. For example:

// 1. Define an EarlyEntryPoint for LoggerImpl
@EarlyEntryPoint
@InstallIn(SingletonComponent.class)
public interface LoggerImplEarlyEntryPoint {
  LoggerImpl getLoggerImpl();
}

// 2. Provide the Logger via EarlyEntryPoints
@Module
@InstallIn(SingletonComponent.class)
interface LoggingModule {
  @Provides
  static Logger provideLogger(Application app) {
    return EarlyEntryPoints.get(app, LoggerImplEarlyEntryPoint.class);
  }
}

Of course, this would still have the same issues about transitive dependencies mentioned before, but it might be enough for your use case.

[kenyee]

hmm..I see...and the logger would implement the Logger and LoggerImplEarlyEntryPoint interfaces.
But this still constrains it so it cannot be replaced in a test?
One other use case we're trying to get working is replacing a featureconfig/toggle class for a test to override values. As I understand it, anything EarlyEntryPoint cannot be replaced by Hilt in a test?

Also, one other quirk I noticed is regular EntryPoints seem to run in Robolectric...is this because testrunners work different in Robolectric vs. E2E/device tests? I.e., the Robolectric tests can override modules earlier than app:onCreate but E2E/device tests cannot because of how APKs and the test APK are loaded/started?

[Chang-Eric]

Just hopping in here, but one thing to note is that using @HiltAndroidTest may not be the best thing for E2E tests. @HiltAndroidTest is really meant for focused tests (I hesitate to say unit tests because people interpret that differently, but basically tests that are more focused on a specific feature or function). For E2E tests, you probably want to be using a regular test that uses the regular Application class without the per-test replacement functionality of @HiltAndroidTest. This could still take the form of a @HiltAndroidApp that is a test application though to replace a few things that don't work well in testing environments.

The difference is E2E tests really shouldn't be trying to replace individual bindings on a per-test level. Like if you want the full realism of running whatever is in your Application startup that does logging and crash reporting and whatnot, it probably isn't a good fit for @HiltAndroidTest.

Also, one other quirk I noticed is regular EntryPoints seem to run in Robolectric...is this because testrunners work different in Robolectric vs. E2E/device tests? I.e., the Robolectric tests can override modules earlier than app:onCreate but E2E/device tests cannot because of how APKs and the test APK are loaded/started?

That's right. Robolectric does not have the same test runner issues as the issues with instrumentation tests described here https://dagger.dev/hilt/early-entry-point. FWIW though, I still would generally not expect Robolectric tests to use a production Application class. Robolectric tests tend to be even smaller, so they really should have even fewer dependencies on needing a specific Application class and should be more feature focused.

[kenyee]

Thanks for confirming Robolectric doesn't have the same issues and it does work well there.

The impetus for this was that we were trying to use Hilt as a way to override feature toggles for specific E2E tests...or replacing dispatchers w/ idling resources while running full E2E tests. We're currently using traditional regular JVM global singleton techniques for this. Sounds like the latter is a better approach given all the pain discovered doing this :-)

I think another technique that might work is to refactor the app startup into another class that app:onCreate() calls. Then this can be called in the production app:onCreate() while in a Hilt E2E test, it gets run in a test rule. That should allow normal Hilt substitution/injection to work while still providing a similar startup?

[Chang-Eric]

I think another technique that might work is to refactor the app startup into another class that app:onCreate() calls. Then this can be called in the production app:onCreate() while in a Hilt E2E test, it gets run in a test rule. That should allow normal Hilt substitution/injection to work while still providing a similar startup?

Yea, I think that would also work since the test rule should be able to get the Context and then you can use https://github.com/google/dagger/blob/master/java/dagger/hilt/android/testing/OnComponentReadyRunner.java

[AlexanderGH]

I just want to clarify explicitly the terminology and confirm what is and isn't supported by @HiltAndroidTest:

• Robolectric tests I think everyone is aligned that they support everything since everything is running in the same JVM process and the context and classloaders are shared between the test code and code under test.
• Then there's the "modular" Hilt tests where the code under test bundles the test modules/entrypoints/etc into the app under test (one per each target in bazel/blaze, or each module in gradle) such that the test can override modules using TestInstallIn, and use things like @BindsValue and other annotations inside the test class. This is basically what is covered in the HiltAndroidTest docs.
• The E2E tests described in the thread above run against a pre-built (and sometimes even proguard obfuscated) app/apk. I just want to confirm whether Hilt attempts to do anything to try and support @HiltAndroid test where the component tree from the app under test is proxied/delegated to the test app such that the test app has full control over the component graph. Currently it's possible as described earlier in this issue to use EntryPoints + OnComponentReady against the target context to peak into the target app's component graph as long as the target app and test app both have the same EntryPoint class available in their classloaders, but I wanted to confirm if @HiltAndroidTest is intended to be supported for this use-case by trying to make the test app's component graph the source of truth.

tldr: A tl;dr of how @HiltAndroidTest works in Gradle device/emulator tests might be useful in a README somewhere to make it easier to see what options are available for integrating into a partially migrated codebase.

[Chang-Eric]

Ah, thanks for the clarification, Alex! I actually kind of missed that that is what's going on with using the OnComponentReadyRunner with @HiltAndroidApp. Actually, since it is entry pointing into the @HiltAndroidApp components, then I don't think you even need the OnComponentReadyRunner since that is waiting on the test components and the @HiltAndroidApp components are ready whenever.

With that said, while it works and we don't really intend to break it, I think this isn't something we are trying to intentionally support. This is what my comment up here was trying to say that for E2E tests you basically should just write those as you would without Hilt. Thanks for catching that and reconfirming it.

tldr: A tl;dr of how @HiltAndroidTest works in Gradle device/emulator tests might be useful in a README somewhere to make it easier to see what options are available for integrating into a partially migrated codebase.

Yea, I agree it would be better to document all this. I'll leave this open for documentation on the testing strategies, though to be honest not sure how soon I'll be able to get to it.

[kenyee]

FWIW, someone at Twitch was trying to do something similar... i.e. lots of E2E tests, that they'd like to add Hilt support for.

We're forced to because, once you make your TestRunner point to a @CustomTestApplication, that requires all your E2E tests to use the Hilt rule as well as the @HiltAndroidTest annotation...would be a lot less painful a migration if these were just warnings. And then we both got into the same app:onCreate() conundrum where we have to make that a "StartupStuff.init()" that's a single line call there and in a new testrule.

Folks who have a legacy codebase w/ E2E tests probably would hit this...

[Chang-Eric]

Sorry, maybe I'm missing something. Is the issue that you have a mix of E2E and smaller @HiltAndroidTest tests, and once you change the TestRunner for the @HiltAndroidTest tests, then it affects the E2E tests and that's why you have this issue? Like if you could set the TestRunner in a way that didn't affect the E2E tests, would that solve your problem?

[AlexanderGH]

Just to clarify: while using EntryPoints to grab things from the app under test works (and you're right, OnComponentReady isn't needed since by the time the test rule/method runs the under-test component is already initialized), I want to confirm @HiltAndroidTest and all its related features are (or are not) supported in the 'it works' sense, not necessarily the 'we officially support this use-case' which i think we're aligned on is not the case.

Edit, i'll give some context since there two related issues here related to handling E2E tests in legacy/migrating codebases:

• the early entrypoint thing is the result of a @CustomTestApplication where the base class in it's super.onCreate (and therefore before hilt creates the singleton component) uses entrypoints. That custom base application seemed to be needed since a lot of the e2e tests assume a lot of eager init has been done in application.onCreate. I think that issue is resolved.
• concurrently, we're trying to see if we can use @HiltAndroidTest + @TestInstallIn to somehow control the dagger graph in a pre-built apk. We can access the app-under-test's dagger graph, but I imagine we can't do anything to somehow redirect the app-under-test's dagger graph to delegate to the test-app's dagger graph?

[Chang-Eric]

concurrently, we're trying to see if we can use @HiltAndroidTest + @TestInstallIn to somehow control the dagger graph in a pre-built apk. We can access the app-under-test's dagger graph, but I imagine we can't do anything to somehow redirect the app-under-test's dagger graph to delegate to the test-app's dagger graph?

Right, I don't think there's anything you can do there if you use a prebuilt Application. The part that "it works" and is I guess supported is that you can access the prebuilt graph using EntryPoints if you share the same classloader. I don't think this will ever break since there's really nothing special about this case. You're just accessing an EntryPoint using the regular mechanism, it just happens to be that code doing the accessing is in the test deps. There's nothing really for redirecting the prebuilt Application though. That should already be basically hard coded into the Application generated by @HiltAndroidApp.