commit f388efa30e9c2d42d2b8e50c32eacf1dbe6d747e
parent 4d2d424e3c1e9fc5df7162cd85d413627998727b
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date: Wed, 5 Nov 2025 10:27:40 +0000
Bug 1996631 - Improve testability of DefaultLocaleSettingsController r=android-reviewers,anpopa,avirvara
This patch refactors the `DefaultLocaleSettingsController` to improve its testability and code clarity. The key changes include:
- Extracting the logic for recreating the activity into a new `recreateActivity()` function.
- Moving the calls to `LocaleManager.setNewLocale` and `LocaleManager.resetToSystemDefault` into new `updateLocale()` and `resetToSystemDefault()` wrapper functions.
- Updating `LocaleSettingsControllerTest` to use Robolectric for a more realistic `Activity` context and adapting the tests to verify the behavior of the newly extracted methods instead of mocking static `LocaleManager` calls.
Differential Revision: https://phabricator.services.mozilla.com/D271359
Diffstat:
3 files changed, 99 insertions(+), 85 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/advanced/DefaultLocaleSettingsController.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/advanced/DefaultLocaleSettingsController.kt
@@ -7,6 +7,7 @@ package org.mozilla.fenix.settings.advanced
import android.app.Activity
import android.content.Context
import android.os.Build
+import androidx.annotation.VisibleForTesting
import mozilla.components.browser.state.action.SearchAction
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.support.locale.LocaleManager
@@ -14,12 +15,49 @@ import mozilla.components.support.locale.LocaleUseCases
import org.mozilla.fenix.nimbus.FxNimbus
import java.util.Locale
+/**
+ * Controller responsible for handling user interactions on the locale settings screen.
+ * This includes selecting a new locale, searching for a locale, and resetting to the
+ * system default locale.
+ */
interface LocaleSettingsController {
+
+ /**
+ * Handles the selection of a new locale.
+ *
+ * @param locale The [Locale] selected by the user.
+ */
fun handleLocaleSelected(locale: Locale)
+
+ /**
+ * Handles user input in the locale search field.
+ *
+ * @param query The search string typed by the user.
+ */
fun handleSearchQueryTyped(query: String)
+
+ /**
+ * Handles the selection of the system's default locale.
+ */
fun handleDefaultLocaleSelected()
}
+/**
+ * Default implementation of [LocaleSettingsController].
+ *
+ * This class manages the logic for changing the application's language. It handles user interactions
+ * from the locale settings screen, such as selecting a new language or resetting to the system default.
+ * It coordinates with various stores and use cases to update the application state, refresh
+ * related components like search engines, and apply the new locale by recreating the activity.
+ *
+ * @param activity The current [Activity] context, required for recreating the UI and accessing resources.
+ * @param localeSettingsStore The store that manages the state for the locale settings UI, such as the
+ * list of available locales and the current search query.
+ * @param browserStore The main browser store, used here to dispatch actions like refreshing search
+ * engines when the locale changes.
+ * @param localeUseCase A set of use cases for managing the application's locale, including setting a new
+ * locale or resetting to the system default.
+ */
class DefaultLocaleSettingsController(
private val activity: Activity,
private val localeSettingsStore: LocaleSettingsStore,
@@ -35,18 +73,12 @@ class DefaultLocaleSettingsController(
}
localeSettingsStore.dispatch(LocaleSettingsAction.Select(locale))
browserStore.dispatch(SearchAction.RefreshSearchEnginesAction)
- LocaleManager.setNewLocale(activity, localeUseCase, locale)
- LocaleManager.updateBaseConfiguration(activity, locale)
+ updateLocale(locale)
+ updateBaseConfiguration(activity, locale)
// Invalidate cached values to use the new locale
FxNimbus.features.nimbusValidation.withCachedValue(null)
- activity.recreate()
- if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
- activity.overrideActivityTransition(Activity.OVERRIDE_TRANSITION_OPEN, 0, 0)
- } else {
- @Suppress("DEPRECATION")
- activity.overridePendingTransition(0, 0)
- }
+ recreateActivity()
}
override fun handleDefaultLocaleSelected() {
@@ -55,11 +87,23 @@ class DefaultLocaleSettingsController(
}
localeSettingsStore.dispatch(LocaleSettingsAction.Select(localeSettingsStore.state.localeList[0]))
browserStore.dispatch(SearchAction.RefreshSearchEnginesAction)
- LocaleManager.resetToSystemDefault(activity, localeUseCase)
- LocaleManager.updateBaseConfiguration(activity, localeSettingsStore.state.localeList[0])
+ resetToSystemDefault()
+ updateBaseConfiguration(activity, localeSettingsStore.state.localeList[0])
// Invalidate cached values to use the default locale
FxNimbus.features.nimbusValidation.withCachedValue(null)
+ recreateActivity()
+ }
+
+ override fun handleSearchQueryTyped(query: String) {
+ localeSettingsStore.dispatch(LocaleSettingsAction.Search(query))
+ }
+
+ /**
+ * Recreates the activity to apply locale changes and provides a smooth, instant transition.
+ */
+ @VisibleForTesting
+ internal fun recreateActivity() {
activity.recreate()
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
activity.overrideActivityTransition(Activity.OVERRIDE_TRANSITION_OPEN, 0, 0)
@@ -69,15 +113,29 @@ class DefaultLocaleSettingsController(
}
}
- override fun handleSearchQueryTyped(query: String) {
- localeSettingsStore.dispatch(LocaleSettingsAction.Search(query))
+ /**
+ * Sets the new application locale and updates the base configuration to apply it.
+ *
+ * @param locale The new [Locale] to set.
+ */
+ @VisibleForTesting
+ internal fun updateLocale(locale: Locale) {
+ LocaleManager.setNewLocale(activity, localeUseCase, locale)
+ }
+
+ /**
+ * Resets the application's locale to the system's default locale.
+ */
+ @VisibleForTesting
+ internal fun resetToSystemDefault() {
+ LocaleManager.resetToSystemDefault(activity, localeUseCase)
}
/**
* Update the locale for the configuration of the app context's resources
*/
@Suppress("Deprecation")
- fun LocaleManager.updateBaseConfiguration(context: Context, locale: Locale) {
+ fun updateBaseConfiguration(context: Context, locale: Locale) {
val resources = context.applicationContext.resources
val config = resources.configuration
config.setLocale(locale)
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleSettingsControllerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleSettingsControllerTest.kt
@@ -5,12 +5,11 @@
package org.mozilla.fenix.settings.advanced
import android.app.Activity
+import androidx.test.ext.junit.runners.AndroidJUnit4
import io.mockk.Runs
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
-import io.mockk.mockkObject
-import io.mockk.mockkStatic
import io.mockk.spyk
import io.mockk.verify
import io.mockk.verifyAll
@@ -20,15 +19,19 @@ import mozilla.components.support.locale.LocaleManager
import mozilla.components.support.locale.LocaleUseCases
import org.junit.Before
import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.Robolectric
import java.util.Locale
+@RunWith(AndroidJUnit4::class)
class LocaleSettingsControllerTest {
- private val activity = mockk<Activity>(relaxed = true)
+ val activity: Activity = Robolectric.buildActivity(Activity::class.java).setup().get()
+
private val localeSettingsStore: LocaleSettingsStore = mockk(relaxed = true)
private val browserStore: BrowserStore = mockk(relaxed = true)
private val localeUseCases: LocaleUseCases = mockk(relaxed = true)
- private val mockState = LocaleSettingsState(mockk(), mockk(), mockk())
+ private val mockState = LocaleSettingsState(emptyList(), emptyList(), mockk())
private lateinit var controller: DefaultLocaleSettingsController
@@ -43,28 +46,24 @@ class LocaleSettingsControllerTest {
),
)
- mockkObject(LocaleManager)
- mockkStatic("org.mozilla.fenix.settings.advanced.LocaleManagerExtensionKt")
+ every { localeUseCases.notifyLocaleChanged(any()) } just Runs
}
@Test
fun `don't set locale if same locale is chosen`() {
val selectedLocale = Locale.Builder().setLanguage("en").setRegion("UK").build()
every { localeSettingsStore.state } returns mockState.copy(selectedLocale = selectedLocale)
- every { LocaleManager.getCurrentLocale(activity) } returns mockk()
+
+ LocaleManager.setNewLocale(activity, locale = selectedLocale)
controller.handleLocaleSelected(selectedLocale)
verifyAll(inverse = true) {
localeSettingsStore.dispatch(LocaleSettingsAction.Select(selectedLocale))
browserStore.dispatch(SearchAction.RefreshSearchEnginesAction)
- LocaleManager.setNewLocale(activity, locale = selectedLocale)
- activity.recreate()
- }
- with(controller) {
- verify(inverse = true) {
- LocaleManager.updateBaseConfiguration(activity, selectedLocale)
- }
+ controller.updateLocale(selectedLocale)
+ controller.recreateActivity()
+ controller.updateBaseConfiguration(activity, selectedLocale)
}
}
@@ -72,100 +71,62 @@ class LocaleSettingsControllerTest {
fun `set a new locale from the list if other locale is chosen`() {
val selectedLocale = Locale.Builder().setLanguage("en").setRegion("UK").build()
val otherLocale: Locale = mockk()
- every { localeUseCases.notifyLocaleChanged } returns mockk()
every { localeSettingsStore.state } returns mockState.copy(selectedLocale = otherLocale)
- every { LocaleManager.setNewLocale(activity, localeUseCases, selectedLocale) } returns activity
- with(controller) {
- every { LocaleManager.updateBaseConfiguration(activity, selectedLocale) } just Runs
- }
controller.handleLocaleSelected(selectedLocale)
verify { localeSettingsStore.dispatch(LocaleSettingsAction.Select(selectedLocale)) }
verify { browserStore.dispatch(SearchAction.RefreshSearchEnginesAction) }
- verify { LocaleManager.setNewLocale(activity, localeUseCases, selectedLocale) }
- verify { activity.recreate() }
- verify {
- @Suppress("DEPRECATION")
- activity.overridePendingTransition(0, 0)
- }
-
- with(controller) {
- verify { LocaleManager.updateBaseConfiguration(activity, selectedLocale) }
- }
+ controller.updateLocale(selectedLocale)
+ controller.recreateActivity()
+ verify { controller.updateBaseConfiguration(activity, selectedLocale) }
}
@Test
fun `set a new locale from the list if default locale is not selected`() {
val selectedLocale = Locale.Builder().setLanguage("en").setRegion("UK").build()
- every { localeUseCases.notifyLocaleChanged } returns mockk()
every { localeSettingsStore.state } returns mockState.copy(selectedLocale = selectedLocale)
- every { LocaleManager.getCurrentLocale(activity) } returns null
- every { LocaleManager.setNewLocale(activity, localeUseCases, selectedLocale) } returns activity
- with(controller) {
- every { LocaleManager.updateBaseConfiguration(activity, selectedLocale) } just Runs
- }
+ LocaleManager.setNewLocale(activity, locale = null)
controller.handleLocaleSelected(selectedLocale)
verify { localeSettingsStore.dispatch(LocaleSettingsAction.Select(selectedLocale)) }
verify { browserStore.dispatch(SearchAction.RefreshSearchEnginesAction) }
- verify { LocaleManager.setNewLocale(activity, localeUseCases, selectedLocale) }
- verify { activity.recreate() }
- verify {
- @Suppress("DEPRECATION")
- activity.overridePendingTransition(0, 0)
- }
-
- with(controller) {
- verify { LocaleManager.updateBaseConfiguration(activity, selectedLocale) }
- }
+ verify { controller.updateLocale(selectedLocale) }
+ verify { controller.recreateActivity() }
+ verify { controller.updateBaseConfiguration(activity, selectedLocale) }
}
@Test
fun `don't set default locale if default locale is already chosen`() {
val selectedLocale = Locale.Builder().setLanguage("en").setRegion("UK").build()
every { localeSettingsStore.state } returns mockState.copy(localeList = listOf(selectedLocale))
- every { LocaleManager.getCurrentLocale(activity) } returns null
+ LocaleManager.setNewLocale(activity, locale = null)
controller.handleDefaultLocaleSelected()
verifyAll(inverse = true) {
localeSettingsStore.dispatch(LocaleSettingsAction.Select(selectedLocale))
browserStore.dispatch(SearchAction.RefreshSearchEnginesAction)
- LocaleManager.resetToSystemDefault(activity, localeUseCases)
- activity.recreate()
- with(controller) {
- LocaleManager.updateBaseConfiguration(activity, selectedLocale)
- }
+ controller.resetToSystemDefault()
+ controller.recreateActivity()
+ controller.updateBaseConfiguration(activity, selectedLocale)
}
}
@Test
fun `set the default locale as the new locale`() {
val selectedLocale = Locale.Builder().setLanguage("en").setRegion("UK").build()
- every { localeUseCases.notifyLocaleChanged } returns mockk()
every { localeSettingsStore.state } returns mockState.copy(localeList = listOf(selectedLocale))
- every { LocaleManager.resetToSystemDefault(activity, localeUseCases) } just Runs
- with(controller) {
- every { LocaleManager.updateBaseConfiguration(activity, selectedLocale) } just Runs
- }
controller.handleDefaultLocaleSelected()
verify { localeSettingsStore.dispatch(LocaleSettingsAction.Select(selectedLocale)) }
verify { browserStore.dispatch(SearchAction.RefreshSearchEnginesAction) }
- verify { LocaleManager.resetToSystemDefault(activity, localeUseCases) }
- verify { activity.recreate() }
- verify {
- @Suppress("DEPRECATION")
- activity.overridePendingTransition(0, 0)
- }
-
- with(controller) {
- verify { LocaleManager.updateBaseConfiguration(activity, selectedLocale) }
- }
+ verify { controller.resetToSystemDefault() }
+ verify { controller.recreateActivity() }
+ verify { controller.updateBaseConfiguration(activity, selectedLocale) }
}
@Test
diff --git a/mobile/android/fenix/config/detekt-baseline.xml b/mobile/android/fenix/config/detekt-baseline.xml
@@ -271,8 +271,6 @@
<ID>UndocumentedPublicClass:DebugDrawerRoute.kt$DebugDrawerRoute$Companion</ID>
<ID>UndocumentedPublicClass:DebugFenixApplication.kt$DebugFenixApplication : FenixApplication</ID>
<ID>UndocumentedPublicClass:DefaultBrowserPreference.kt$DefaultBrowserPreference : Preference</ID>
- <ID>UndocumentedPublicClass:DefaultLocaleSettingsController.kt$DefaultLocaleSettingsController : LocaleSettingsController</ID>
- <ID>UndocumentedPublicClass:DefaultLocaleSettingsController.kt$LocaleSettingsController</ID>
<ID>UndocumentedPublicClass:DefaultSyncController.kt$SyncController</ID>
<ID>UndocumentedPublicClass:DefaultSyncInteractor.kt$SyncInteractor</ID>
<ID>UndocumentedPublicClass:DefaultTopSitesView.kt$DefaultTopSitesView : TopSitesView</ID>
@@ -667,9 +665,6 @@
<ID>UndocumentedPublicFunction:CounterPreference.kt$CounterPreference$fun underMaxCount()</ID>
<ID>UndocumentedPublicFunction:CreditCardItemViewHolder.kt$CreditCardItemViewHolder$fun bind(creditCard: CreditCard)</ID>
<ID>UndocumentedPublicFunction:DefaultBrowserPreference.kt$DefaultBrowserPreference$fun updateSwitch()</ID>
- <ID>UndocumentedPublicFunction:DefaultLocaleSettingsController.kt$LocaleSettingsController$fun handleDefaultLocaleSelected()</ID>
- <ID>UndocumentedPublicFunction:DefaultLocaleSettingsController.kt$LocaleSettingsController$fun handleLocaleSelected(locale: Locale)</ID>
- <ID>UndocumentedPublicFunction:DefaultLocaleSettingsController.kt$LocaleSettingsController$fun handleSearchQueryTyped(query: String)</ID>
<ID>UndocumentedPublicFunction:DefaultSyncController.kt$SyncController$fun handleCameraPermissionsNeeded()</ID>
<ID>UndocumentedPublicFunction:DefaultSyncInteractor.kt$SyncInteractor$fun onCameraPermissionsNeeded()</ID>
<ID>UndocumentedPublicFunction:DefaultToolbarMenu.kt$DefaultToolbarMenu$@VisibleForTesting(otherwise = PRIVATE) fun canAddToHomescreen(): Boolean</ID>
