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

Use DI for AccountSettings and Android tests #911

Merged
merged 7 commits into from
Jul 15, 2024
Merged

Conversation

rfc2822
Copy link
Member

@rfc2822 rfc2822 commented Jul 15, 2024

  • Use Hilt for AccountSettings.
  • Use Hilt for Android tests when things need to be injected.

Findings:

  • There's no guaranteed order for multiple @Before methods in JUnit tests, so we should only have one @Before fun setup() per test class. This setup method runs AndroidHiltRule.inject() first and can then initialize other things. Test variables that depend on the Context need to be declared by lazy because the Context will only be available after the Hilt rule is run.
  • In Android tests using Hilt (@HiltAndroidTest), we should always inject the application context instead of getting it from the instrumentation. Otherwise Hilt may have stored another (wrapped or whatever) Application as application context and then things like EntryPointAccessors.fromApplication won't work properly.
  • We should always inject @ApplicationContext val context: Context instead of an Application for the same reason.

@rfc2822 rfc2822 added the refactoring Internal improvement of existing functions label Jul 15, 2024
@rfc2822 rfc2822 self-assigned this Jul 15, 2024
@rfc2822 rfc2822 force-pushed the accountsettings-di branch from 30e89fe to f04f77b Compare July 15, 2024 15:23
@rfc2822 rfc2822 force-pushed the accountsettings-di branch from f04f77b to bfd3ed9 Compare July 15, 2024 15:35
@rfc2822 rfc2822 force-pushed the accountsettings-di branch from 7de9287 to ba6dffb Compare July 15, 2024 16:56
@rfc2822 rfc2822 changed the title Use DI for AccountSettings Use DI for AccountSettings and Android tests Jul 15, 2024
@rfc2822 rfc2822 marked this pull request as ready for review July 15, 2024 17:33
@rfc2822 rfc2822 merged commit 1fd65a4 into main-ose Jul 15, 2024
11 checks passed
@rfc2822 rfc2822 deleted the accountsettings-di branch July 15, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Internal improvement of existing functions
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

1 participant