commit dab8a944f21021d563ddf418f5a6db9a647e836f
parent 88f2bd36ece7c0cc999d565605e402893f419390
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date: Wed, 10 Dec 2025 14:16:29 +0000
Bug 2005186 - Refactor DefaultTopSitesBindingTest to use StandardTestDispatcher r=android-reviewers,avirvara
This patch replaces the usage of `MainCoroutineRule` and `runTestOnMain` with `StandardTestDispatcher` and `runTest` in `DefaultTopSitesBindingTest`.
Removed the hard dependency on `Dispatchers.IO` within `getTopSites`.
Differential Revision: https://phabricator.services.mozilla.com/D275777
Diffstat:
2 files changed, 187 insertions(+), 169 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/topsites/DefaultTopSitesBinding.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/topsites/DefaultTopSitesBinding.kt
@@ -5,6 +5,7 @@
package org.mozilla.fenix.home.topsites
import android.content.res.Resources
+import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.distinctUntilChanged
@@ -36,6 +37,9 @@ import org.mozilla.fenix.utils.Settings
* @param resources [Resources] used for accessing application resources.
* @param crashReporter [CrashReporter] used for recording caught exceptions.
* @param isReleased Whether or not the build is in a release channel.
+ * @param mainDispatcher The dispatcher on which to observe state changes.
+ * @param ioDispatcher The dispatcher used for I/O operations,
+ * specifically reading and parsing the initial shortcuts JSON.
*/
class DefaultTopSitesBinding(
browserStore: BrowserStore,
@@ -44,7 +48,9 @@ class DefaultTopSitesBinding(
private val resources: Resources,
private val crashReporter: CrashReporter,
private val isReleased: Boolean = Config.channel.isReleased,
-) : AbstractBinding<BrowserState>(browserStore) {
+ mainDispatcher: CoroutineDispatcher = Dispatchers.Main,
+ private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
+) : AbstractBinding<BrowserState>(browserStore, mainDispatcher = mainDispatcher) {
override suspend fun onState(flow: Flow<BrowserState>) {
if (settings.defaultTopSitesAdded) return
@@ -66,7 +72,7 @@ class DefaultTopSitesBinding(
}
}
- internal suspend fun getTopSites(region: String): List<Pair<String, String>> = withContext(Dispatchers.IO) {
+ internal suspend fun getTopSites(region: String): List<Pair<String, String>> = withContext(ioDispatcher) {
try {
val json = Json { ignoreUnknownKeys = true }
val jsonString = resources.openRawResource(R.raw.initial_shortcuts).bufferedReader()
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/topsites/DefaultTopSitesBindingTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/topsites/DefaultTopSitesBindingTest.kt
@@ -9,6 +9,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
+import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.runTest
import kotlinx.serialization.SerializationException
import mozilla.components.browser.state.action.SearchAction
@@ -16,12 +17,9 @@ import mozilla.components.browser.state.search.RegionState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.feature.top.sites.DefaultTopSitesStorage
import mozilla.components.lib.crash.CrashReporter
-import mozilla.components.support.test.rule.MainCoroutineRule
-import mozilla.components.support.test.rule.runTestOnMain
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Before
-import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.R
@@ -30,8 +28,6 @@ import java.io.ByteArrayInputStream
@RunWith(AndroidJUnit4::class)
class DefaultTopSitesBindingTest {
- @get:Rule
- val coroutineRule = MainCoroutineRule()
private lateinit var browserStore: BrowserStore
private lateinit var topSitesStorage: DefaultTopSitesStorage
@@ -39,6 +35,8 @@ class DefaultTopSitesBindingTest {
private lateinit var resources: Resources
private lateinit var crashReporter: CrashReporter
+ private val dispatcher = StandardTestDispatcher()
+
@Before
fun setUp() {
topSitesStorage = mockk(relaxed = true)
@@ -56,203 +54,215 @@ class DefaultTopSitesBindingTest {
}
@Test
- fun `GIVEN build is in the release channel WHEN region is set to the default THEN do nothing`() = runTestOnMain {
- every { settings.defaultTopSitesAdded } returns false
+ fun `GIVEN build is in the release channel WHEN region is set to the default THEN do nothing`() =
+ runTest(dispatcher) {
+ every { settings.defaultTopSitesAdded } returns false
- val binding = createBinding()
- binding.start()
+ val binding = createBinding()
+ binding.start()
- browserStore.dispatch(
- SearchAction.SetRegionAction(RegionState.Default),
- )
+ browserStore.dispatch(
+ SearchAction.SetRegionAction(RegionState.Default),
+ )
- verify(exactly = 0) {
- topSitesStorage.addTopSites(topSites = any(), isDefault = any())
- settings.defaultTopSitesAdded = any()
+ verify(exactly = 0) {
+ topSitesStorage.addTopSites(topSites = any(), isDefault = any())
+ settings.defaultTopSitesAdded = any()
+ }
}
- }
@Test
- fun `GIVEN debug build WHEN region is set to the default THEN add the default sites to storage`() = runTestOnMain {
- every { settings.defaultTopSitesAdded } returns false
+ fun `GIVEN debug build WHEN region is set to the default THEN add the default sites to storage`() =
+ runTest(dispatcher) {
+ every { settings.defaultTopSitesAdded } returns false
- val binding = createBinding(isReleased = false)
- binding.start()
+ val binding = createBinding(isReleased = false)
+ binding.start()
- browserStore.dispatch(
- SearchAction.SetRegionAction(RegionState.Default),
- )
+ browserStore.dispatch(
+ SearchAction.SetRegionAction(RegionState.Default),
+ )
- val topSites = binding.getTopSites(region = "XX")
+ val topSites = binding.getTopSites(region = "XX")
+ dispatcher.scheduler.advanceUntilIdle()
- verify {
- topSitesStorage.addTopSites(topSites = topSites, isDefault = true)
- settings.defaultTopSitesAdded = true
+ verify {
+ topSitesStorage.addTopSites(topSites = topSites, isDefault = true)
+ settings.defaultTopSitesAdded = true
+ }
}
- }
@Test
- fun `WHEN region is set to a non-default value THEN add the sites for that region to storage`() = runTestOnMain {
- every { settings.defaultTopSitesAdded } returns false
+ fun `WHEN region is set to a non-default value THEN add the sites for that region to storage`() =
+ runTest(dispatcher) {
+ every { settings.defaultTopSitesAdded } returns false
- val region = "CA"
- val binding = createBinding()
- binding.start()
+ val region = "CA"
+ val binding = createBinding()
+ binding.start()
- browserStore.dispatch(
- SearchAction.SetRegionAction(RegionState(home = region, current = region)),
- )
+ browserStore.dispatch(
+ SearchAction.SetRegionAction(RegionState(home = region, current = region)),
+ )
- val topSites = binding.getTopSites(region = region)
+ val topSites = binding.getTopSites(region = region)
+ dispatcher.scheduler.advanceUntilIdle()
- verify {
- topSitesStorage.addTopSites(topSites = topSites, isDefault = true)
- settings.defaultTopSitesAdded = true
+ verify {
+ topSitesStorage.addTopSites(topSites = topSites, isDefault = true)
+ settings.defaultTopSitesAdded = true
+ }
}
- }
@Test
- fun `GIVEN default top sites have already been added WHEN region is set to a non-default value THEN do nothing`() = runTestOnMain {
- every { settings.defaultTopSitesAdded } returns true
-
- val region = "CA"
- val binding = createBinding()
- binding.start()
-
- browserStore.dispatch(
- SearchAction.SetRegionAction(RegionState(home = region, current = region)),
- )
-
- verify(exactly = 0) {
- topSitesStorage.addTopSites(topSites = any(), isDefault = any())
- settings.defaultTopSitesAdded = any()
+ fun `GIVEN default top sites have already been added WHEN region is set to a non-default value THEN do nothing`() =
+ runTest(dispatcher) {
+ every { settings.defaultTopSitesAdded } returns true
+
+ val region = "CA"
+ val binding = createBinding()
+ binding.start()
+
+ browserStore.dispatch(
+ SearchAction.SetRegionAction(RegionState(home = region, current = region)),
+ )
+
+ verify(exactly = 0) {
+ topSitesStorage.addTopSites(topSites = any(), isDefault = any())
+ settings.defaultTopSitesAdded = any()
+ }
}
- }
@Test
- fun `GIVEN region is in an included region WHEN getTopSites is called THEN the sites for that region are returned`() = runTest {
- val binding = createBinding()
- val topSites = binding.getTopSites(region = "US")
-
- assertEquals(7, topSites.size)
- assertEquals("US Region Site", topSites[0].first)
- assertEquals("https://www.example1.com/", topSites[0].second)
- assertEquals("CA Excluded Region Site", topSites[1].first)
- assertEquals("https://www.example2.com/", topSites[1].second)
- assertEquals("All Region Site", topSites[2].first)
- assertEquals("https://www.example3.com/", topSites[2].second)
- assertEquals("www.example4.com", topSites[3].first)
- assertEquals("https://www.example4.com/", topSites[3].second)
- assertEquals("www.example5.com", topSites[4].first)
- assertEquals("https://www.example5.com/", topSites[4].second)
- assertEquals("www.example6.com", topSites[5].first)
- assertEquals("https://www.example6.com/", topSites[5].second)
- assertEquals("www.example7.com", topSites[6].first)
- assertEquals("https://www.example7.com/", topSites[6].second)
- }
+ fun `GIVEN region is in an included region WHEN getTopSites is called THEN the sites for that region are returned`() =
+ runTest(dispatcher) {
+ val binding = createBinding()
+ val topSites = binding.getTopSites(region = "US")
+
+ assertEquals(7, topSites.size)
+ assertEquals("US Region Site", topSites[0].first)
+ assertEquals("https://www.example1.com/", topSites[0].second)
+ assertEquals("CA Excluded Region Site", topSites[1].first)
+ assertEquals("https://www.example2.com/", topSites[1].second)
+ assertEquals("All Region Site", topSites[2].first)
+ assertEquals("https://www.example3.com/", topSites[2].second)
+ assertEquals("www.example4.com", topSites[3].first)
+ assertEquals("https://www.example4.com/", topSites[3].second)
+ assertEquals("www.example5.com", topSites[4].first)
+ assertEquals("https://www.example5.com/", topSites[4].second)
+ assertEquals("www.example6.com", topSites[5].first)
+ assertEquals("https://www.example6.com/", topSites[5].second)
+ assertEquals("www.example7.com", topSites[6].first)
+ assertEquals("https://www.example7.com/", topSites[6].second)
+ }
@Test
- fun `GIVEN region is in an included region and Japan default site experiment is turned off WHEN getTopSites is called THEN the sites for that region are returned`() = runTest {
- every { settings.showFirefoxJpGuideDefaultSite } returns false
-
- val binding = createBinding()
- val topSites = binding.getTopSites(region = "US")
-
- assertEquals(6, topSites.size)
- assertEquals("US Region Site", topSites[0].first)
- assertEquals("https://www.example1.com/", topSites[0].second)
- assertEquals("CA Excluded Region Site", topSites[1].first)
- assertEquals("https://www.example2.com/", topSites[1].second)
- assertEquals("All Region Site", topSites[2].first)
- assertEquals("https://www.example3.com/", topSites[2].second)
- assertEquals("www.example4.com", topSites[3].first)
- assertEquals("https://www.example4.com/", topSites[3].second)
- assertEquals("www.example5.com", topSites[4].first)
- assertEquals("https://www.example5.com/", topSites[4].second)
- assertEquals("www.example7.com", topSites[5].first)
- assertEquals("https://www.example7.com/", topSites[5].second)
- }
+ fun `GIVEN region is in an included region and Japan default site experiment is turned off WHEN getTopSites is called THEN the sites for that region are returned`() =
+ runTest(dispatcher) {
+ every { settings.showFirefoxJpGuideDefaultSite } returns false
+
+ val binding = createBinding()
+ val topSites = binding.getTopSites(region = "US")
+
+ assertEquals(6, topSites.size)
+ assertEquals("US Region Site", topSites[0].first)
+ assertEquals("https://www.example1.com/", topSites[0].second)
+ assertEquals("CA Excluded Region Site", topSites[1].first)
+ assertEquals("https://www.example2.com/", topSites[1].second)
+ assertEquals("All Region Site", topSites[2].first)
+ assertEquals("https://www.example3.com/", topSites[2].second)
+ assertEquals("www.example4.com", topSites[3].first)
+ assertEquals("https://www.example4.com/", topSites[3].second)
+ assertEquals("www.example5.com", topSites[4].first)
+ assertEquals("https://www.example5.com/", topSites[4].second)
+ assertEquals("www.example7.com", topSites[5].first)
+ assertEquals("https://www.example7.com/", topSites[5].second)
+ }
@Test
- fun `GIVEN region is in an excluded region WHEN getTopSites is called THEN the sites for that region are not returned`() = runTest {
- val binding = createBinding()
- val topSites = binding.getTopSites(region = "CA")
-
- assertEquals(5, topSites.size)
- assertEquals("All Region Site", topSites[0].first)
- assertEquals("https://www.example3.com/", topSites[0].second)
- assertEquals("www.example4.com", topSites[1].first)
- assertEquals("https://www.example4.com/", topSites[1].second)
- assertEquals("www.example5.com", topSites[2].first)
- assertEquals("https://www.example5.com/", topSites[2].second)
- assertEquals("www.example6.com", topSites[3].first)
- assertEquals("https://www.example6.com/", topSites[3].second)
- assertEquals("www.example7.com", topSites[4].first)
- assertEquals("https://www.example7.com/", topSites[4].second)
- }
+ fun `GIVEN region is in an excluded region WHEN getTopSites is called THEN the sites for that region are not returned`() =
+ runTest(dispatcher) {
+ val binding = createBinding()
+ val topSites = binding.getTopSites(region = "CA")
+
+ assertEquals(5, topSites.size)
+ assertEquals("All Region Site", topSites[0].first)
+ assertEquals("https://www.example3.com/", topSites[0].second)
+ assertEquals("www.example4.com", topSites[1].first)
+ assertEquals("https://www.example4.com/", topSites[1].second)
+ assertEquals("www.example5.com", topSites[2].first)
+ assertEquals("https://www.example5.com/", topSites[2].second)
+ assertEquals("www.example6.com", topSites[3].first)
+ assertEquals("https://www.example6.com/", topSites[3].second)
+ assertEquals("www.example7.com", topSites[4].first)
+ assertEquals("https://www.example7.com/", topSites[4].second)
+ }
@Test
- fun `GIVEN the default region region WHEN getTopSites is called THEN the sites for that region are returned`() = runTest {
- val binding = createBinding()
- val topSites = binding.getTopSites(region = "XX")
-
- assertEquals(6, topSites.size)
- assertEquals("CA Excluded Region Site", topSites[0].first)
- assertEquals("https://www.example2.com/", topSites[0].second)
- assertEquals("All Region Site", topSites[1].first)
- assertEquals("https://www.example3.com/", topSites[1].second)
- assertEquals("www.example4.com", topSites[2].first)
- assertEquals("https://www.example4.com/", topSites[2].second)
- assertEquals("www.example5.com", topSites[3].first)
- assertEquals("https://www.example5.com/", topSites[3].second)
- assertEquals("www.example6.com", topSites[4].first)
- assertEquals("https://www.example6.com/", topSites[4].second)
- assertEquals("www.example7.com", topSites[5].first)
- assertEquals("https://www.example7.com/", topSites[5].second)
- }
+ fun `GIVEN the default region region WHEN getTopSites is called THEN the sites for that region are returned`() =
+ runTest(dispatcher) {
+ val binding = createBinding()
+ val topSites = binding.getTopSites(region = "XX")
+
+ assertEquals(6, topSites.size)
+ assertEquals("CA Excluded Region Site", topSites[0].first)
+ assertEquals("https://www.example2.com/", topSites[0].second)
+ assertEquals("All Region Site", topSites[1].first)
+ assertEquals("https://www.example3.com/", topSites[1].second)
+ assertEquals("www.example4.com", topSites[2].first)
+ assertEquals("https://www.example4.com/", topSites[2].second)
+ assertEquals("www.example5.com", topSites[3].first)
+ assertEquals("https://www.example5.com/", topSites[3].second)
+ assertEquals("www.example6.com", topSites[4].first)
+ assertEquals("https://www.example6.com/", topSites[4].second)
+ assertEquals("www.example7.com", topSites[5].first)
+ assertEquals("https://www.example7.com/", topSites[5].second)
+ }
@Test
- fun `GIVEN invalid json WHEN getTopSites is called THEN return empty list and report crash`() = runTest {
- val malformedJson =
- "{\"data\": [{\"url\": \"https://example.com\", \"title\": \"Valid\"}, {\"id\": \"invalid\"}]}"
- every { resources.openRawResource(R.raw.initial_shortcuts) } returns ByteArrayInputStream(
- malformedJson.toByteArray(),
- )
-
- val binding = createBinding()
- val topSites = binding.getTopSites(region = "XX")
-
- assertTrue(topSites.isEmpty())
- verify {
- crashReporter.recordCrashBreadcrumb(any())
- crashReporter.submitCaughtException(any<SerializationException>())
- }
- verify(exactly = 0) {
- topSitesStorage.addTopSites(topSites = any(), isDefault = any())
- settings.defaultTopSitesAdded = any()
+ fun `GIVEN invalid json WHEN getTopSites is called THEN return empty list and report crash`() =
+ runTest(dispatcher) {
+ val malformedJson =
+ "{\"data\": [{\"url\": \"https://example.com\", \"title\": \"Valid\"}, {\"id\": \"invalid\"}]}"
+ every { resources.openRawResource(R.raw.initial_shortcuts) } returns ByteArrayInputStream(
+ malformedJson.toByteArray(),
+ )
+
+ val binding = createBinding()
+ val topSites = binding.getTopSites(region = "XX")
+
+ assertTrue(topSites.isEmpty())
+ verify {
+ crashReporter.recordCrashBreadcrumb(any())
+ crashReporter.submitCaughtException(any<SerializationException>())
+ }
+ verify(exactly = 0) {
+ topSitesStorage.addTopSites(topSites = any(), isDefault = any())
+ settings.defaultTopSitesAdded = any()
+ }
}
- }
@Test
- fun `GIVEN malformed json WHEN getTopSites is called THEN return empty list and report crash for illegal argument`() = runTest {
- val malformedJson = "this is not a valid json"
- every { resources.openRawResource(R.raw.initial_shortcuts) } returns ByteArrayInputStream(
- malformedJson.toByteArray(),
- )
-
- val binding = createBinding()
- val topSites = binding.getTopSites(region = "XX")
-
- assertTrue(topSites.isEmpty())
- verify {
- crashReporter.recordCrashBreadcrumb(any())
- crashReporter.submitCaughtException(any<SerializationException>())
+ fun `GIVEN malformed json WHEN getTopSites is called THEN return empty list and report crash for illegal argument`() =
+ runTest(dispatcher) {
+ val malformedJson = "this is not a valid json"
+ every { resources.openRawResource(R.raw.initial_shortcuts) } returns ByteArrayInputStream(
+ malformedJson.toByteArray(),
+ )
+
+ val binding = createBinding()
+ val topSites = binding.getTopSites(region = "XX")
+
+ assertTrue(topSites.isEmpty())
+ verify {
+ crashReporter.recordCrashBreadcrumb(any())
+ crashReporter.submitCaughtException(any<SerializationException>())
+ }
+ verify(exactly = 0) {
+ topSitesStorage.addTopSites(topSites = any(), isDefault = any())
+ settings.defaultTopSitesAdded = any()
+ }
}
- verify(exactly = 0) {
- topSitesStorage.addTopSites(topSites = any(), isDefault = any())
- settings.defaultTopSitesAdded = any()
- }
- }
private fun createBinding(
isReleased: Boolean = true,
@@ -263,5 +273,7 @@ class DefaultTopSitesBindingTest {
resources = resources,
crashReporter = crashReporter,
isReleased = isReleased,
+ mainDispatcher = dispatcher,
+ ioDispatcher = dispatcher,
)
}
