tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 89c4b1d36f8fde5ff7f9a47be9d360327f77eac9
parent 2fbbbb9f2107d8c8a385f98962c0b0d8dfcab1b0
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date:   Tue,  7 Oct 2025 15:35:47 +0000

Bug 1889427 - Migrate TelemetryMiddlewareTest to runTest. r=android-reviewers,giorga

This patch migrates the `TelemetryMiddlewareTest` from using the deprecated `MainCoroutineRule` to `runTest` for managing coroutines in tests.

The changes include:
- Removing the `MainCoroutineRule`.
- Wrapping each test body in a `runTest` block.
- Removing `EngineMiddleware` from the test setup as it was not required by the tests.
- Update test for killing 50 tabs to avoid excess logging for killing unexisting sessions.

Differential Revision: https://phabricator.services.mozilla.com/D267784

Diffstat:
Mmobile/android/fenix/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt | 359+++++++++++++++++++++++++++++++++++++++++++++----------------------------------
1 file changed, 204 insertions(+), 155 deletions(-)

diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt @@ -10,12 +10,12 @@ import io.mockk.just import io.mockk.mockk import io.mockk.runs import io.mockk.verify +import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.EngineAction import mozilla.components.browser.state.action.ExtensionsProcessAction import mozilla.components.browser.state.action.TabListAction import mozilla.components.browser.state.action.TranslationsAction -import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.state.recover.RecoverableTab @@ -24,10 +24,10 @@ import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.translate.TranslationError import mozilla.components.concept.engine.translate.TranslationOperation +import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.support.base.android.Clock import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.robolectric.testContext -import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -60,9 +60,7 @@ class TelemetryMiddlewareTest { private lateinit var appStore: AppStore private lateinit var settings: Settings private lateinit var telemetryMiddleware: TelemetryMiddleware - - @get:Rule - val coroutinesTestRule = MainCoroutineRule() + private lateinit var tabsUseCases: TabsUseCases @get:Rule val gleanRule = FenixGleanTestRule(ApplicationProvider.getApplicationContext()) @@ -87,11 +85,12 @@ class TelemetryMiddlewareTest { every { engine.createSession(any(), any()) } returns mockk(relaxed = true) store = BrowserStore( - middleware = listOf(telemetryMiddleware) + EngineMiddleware.create(engine), + middleware = listOf(telemetryMiddleware), initialState = BrowserState(), ) appStore = AppStore() every { testContext.components.appStore } returns appStore + tabsUseCases = TabsUseCases(store) } @After @@ -100,7 +99,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN a tab is added THEN the open tab count is updated`() { + fun `WHEN a tab is added THEN the open tab count is updated`() = runTest { assertEquals(0, settings.openTabsCount) assertNull(Metrics.hasOpenTabs.testGetValue()) @@ -111,18 +110,19 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN a private tab is added THEN the open tab count is not updated`() { + fun `WHEN a private tab is added THEN the open tab count is not updated`() = runTest { assertEquals(0, settings.openTabsCount) assertNull(Metrics.hasOpenTabs.testGetValue()) - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org", private = true))).joinBlocking() + store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org", private = true))) + .joinBlocking() assertEquals(0, settings.openTabsCount) assertFalse(Metrics.hasOpenTabs.testGetValue()!!) } @Test - fun `WHEN multiple tabs are added THEN the open tab count is updated`() { + fun `WHEN multiple tabs are added THEN the open tab count is updated`() = runTest { assertEquals(0, settings.openTabsCount) assertNull(Metrics.hasOpenTabs.testGetValue()) @@ -141,7 +141,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN a tab is removed THEN the open tab count is updated`() { + fun `WHEN a tab is removed THEN the open tab count is updated`() = runTest { assertNull(Metrics.hasOpenTabs.testGetValue()) store.dispatch( @@ -161,7 +161,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN all tabs are removed THEN the open tab count is updated`() { + fun `WHEN all tabs are removed THEN the open tab count is updated`() = runTest { assertNull(Metrics.hasOpenTabs.testGetValue()) store.dispatch( @@ -183,7 +183,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN all normal tabs are removed THEN the open tab count is updated`() { + fun `WHEN all normal tabs are removed THEN the open tab count is updated`() = runTest { assertNull(Metrics.hasOpenTabs.testGetValue()) store.dispatch( @@ -204,7 +204,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN tabs are restored THEN the open tab count is updated`() { + fun `WHEN tabs are restored THEN the open tab count is updated`() = runTest { assertEquals(0, settings.openTabsCount) assertNull(Metrics.hasOpenTabs.testGetValue()) @@ -225,40 +225,52 @@ class TelemetryMiddlewareTest { } @Test - fun `GIVEN a normal page is loading WHEN loading is complete THEN we record a UriOpened event`() { - val tab = createTab(id = "1", url = "https://mozilla.org") - assertNull(Events.normalAndPrivateUriCount.testGetValue()) - - store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() - store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, true)).joinBlocking() - assertNull(Events.normalAndPrivateUriCount.testGetValue()) - - store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, false)).joinBlocking() - val count = Events.normalAndPrivateUriCount.testGetValue()!! - assertEquals(1, count) - } + fun `GIVEN a normal page is loading WHEN loading is complete THEN we record a UriOpened event`() = + runTest { + val tab = createTab(id = "1", url = "https://mozilla.org") + assertNull(Events.normalAndPrivateUriCount.testGetValue()) + + store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() + store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, true)).joinBlocking() + assertNull(Events.normalAndPrivateUriCount.testGetValue()) + + store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, false)).joinBlocking() + val count = Events.normalAndPrivateUriCount.testGetValue()!! + assertEquals(1, count) + } @Test - fun `GIVEN a private page is loading WHEN loading is complete THEN we record a UriOpened event`() { - val tab = createTab(id = "1", url = "https://mozilla.org", private = true) - assertNull(Events.normalAndPrivateUriCount.testGetValue()) - - store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() - store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, true)).joinBlocking() - assertNull(Events.normalAndPrivateUriCount.testGetValue()) - - store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, false)).joinBlocking() - val count = Events.normalAndPrivateUriCount.testGetValue()!! - assertEquals(1, count) - } + fun `GIVEN a private page is loading WHEN loading is complete THEN we record a UriOpened event`() = + runTest { + val tab = createTab(id = "1", url = "https://mozilla.org", private = true) + assertNull(Events.normalAndPrivateUriCount.testGetValue()) + + store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() + store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, true)).joinBlocking() + assertNull(Events.normalAndPrivateUriCount.testGetValue()) + + store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, false)).joinBlocking() + val count = Events.normalAndPrivateUriCount.testGetValue()!! + assertEquals(1, count) + } @Test - fun `WHEN tabs gets killed THEN middleware sends an event`() { + fun `WHEN tabs gets killed THEN middleware sends an event`() = runTest { store.dispatch( TabListAction.RestoreAction( listOf( - RecoverableTab(null, TabState(url = "https://www.mozilla.org", id = "foreground")), - RecoverableTab(null, TabState(url = "https://getpocket.com", id = "background_pocket", hasFormData = true)), + RecoverableTab( + null, + TabState(url = "https://www.mozilla.org", id = "foreground"), + ), + RecoverableTab( + null, + TabState( + url = "https://getpocket.com", + id = "background_pocket", + hasFormData = true, + ), + ), ), selectedTabId = "foreground", restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING, @@ -295,24 +307,35 @@ class TelemetryMiddlewareTest { } @Test - fun `GIVEN the request to check for form data WHEN it fails THEN telemetry is sent`() { - assertNull(Events.formDataFailure.testGetValue()) + fun `GIVEN the request to check for form data WHEN it fails THEN telemetry is sent`() = + runTest { + assertNull(Events.formDataFailure.testGetValue()) + + store.dispatch( + ContentAction.CheckForFormDataExceptionAction( + "1", + RuntimeException("session form data request failed"), + ), + ).joinBlocking() - store.dispatch( - ContentAction.CheckForFormDataExceptionAction("1", RuntimeException("session form data request failed")), - ).joinBlocking() + // Wait for the main looper to process the re-thrown exception. + ShadowLooper.idleMainLooper() - // Wait for the main looper to process the re-thrown exception. - ShadowLooper.idleMainLooper() - - assertNotNull(Events.formDataFailure.testGetValue()) - } + assertNotNull(Events.formDataFailure.testGetValue()) + } @Test - fun `GIVEN an existing tab WHEN it reloads THEN telemetry is sent`() { + fun `GIVEN an existing tab WHEN it reloads THEN telemetry is sent`() = runTest { val tabId = "test-tab-id" - store.dispatch(TabListAction.AddTabAction(createTab(id = tabId, url = "https://firefox.com"))) + store.dispatch( + TabListAction.AddTabAction( + createTab( + id = tabId, + url = "https://firefox.com", + ), + ), + ) .joinBlocking() store.dispatch( @@ -334,81 +357,101 @@ class TelemetryMiddlewareTest { } @Test - fun `GIVEN a tab that was not recently killed WHEN it reloads THEN telemetry is NOT sent`() { - val tabId = "test-tab-id" + fun `GIVEN a tab that was not recently killed WHEN it reloads THEN telemetry is NOT sent`() = + runTest { + val tabId = "test-tab-id" - store.dispatch( - TabListAction.AddTabAction(createTab(id = tabId, url = "https://firefox.com")), - ).joinBlocking() + store.dispatch( + TabListAction.AddTabAction(createTab(id = tabId, url = "https://firefox.com")), + ).joinBlocking() - store.dispatch( - EngineAction.CreateEngineSessionAction(tabId), - ).joinBlocking() + store.dispatch( + EngineAction.CreateEngineSessionAction(tabId), + ).joinBlocking() - ShadowLooper.idleMainLooper() + ShadowLooper.idleMainLooper() - val recordedEvents = EngineMetrics.reloaded.testGetValue() - assertTrue(recordedEvents.isNullOrEmpty()) - } + val recordedEvents = EngineMetrics.reloaded.testGetValue() + assertTrue(recordedEvents.isNullOrEmpty()) + } @Test - fun `GIVEN a tab that is killed multiple times WHEN checking recentlyKilledTabs THEN it only appears once`() { - val tabId = "test-tab-id" + fun `GIVEN a tab that is killed multiple times WHEN checking recentlyKilledTabs THEN it only appears once`() = + runTest { + val tabId = "test-tab-id" - store.dispatch(EngineAction.KillEngineSessionAction(tabId)).joinBlocking() - store.dispatch(EngineAction.KillEngineSessionAction(tabId)).joinBlocking() + store.dispatch(EngineAction.KillEngineSessionAction(tabId)).joinBlocking() + store.dispatch(EngineAction.KillEngineSessionAction(tabId)).joinBlocking() - assertEquals(1, store.state.recentlyKilledTabs.count { it == tabId }) - } + assertEquals(1, store.state.recentlyKilledTabs.count { it == tabId }) + } @Test - fun `GIVEN more than 50 tabs are killed WHEN checking recentlyKilledTabs THEN it does not exceed 50`() { - repeat(51) { i -> - store.dispatch(EngineAction.KillEngineSessionAction("tab-$i")).joinBlocking() + fun `GIVEN more than 50 tabs are killed WHEN checking recentlyKilledTabs THEN it does not exceed 50`() = + runTest { + repeat(51) { i -> + val tab = createTab("https://www.mozilla.org") + store.dispatch(TabListAction.AddTabAction(tab)) + store.dispatch(EngineAction.KillEngineSessionAction(tab.id)).joinBlocking() + } + + assertEquals(50, store.state.recentlyKilledTabs.size) } - assertEquals(50, store.state.recentlyKilledTabs.size) - } - @Test - fun `GIVEN 50 killed tabs WHEN another killed tab is reloaded THEN oldest tab is removed and reloaded tab is recorded`() { - val oldestTabId = "tab-id-0" - val newTabId = "new-tab-id" - - // Fill recentlyKilledTabs with 50 entries and verify max limit is reached - repeat(50) { i -> - val tabId = "tab-id-$i" - store.dispatch(TabListAction.AddTabAction(createTab(id = tabId, url = "https://example.com/$i"))).joinBlocking() - store.dispatch(EngineAction.KillEngineSessionAction(tabId)).joinBlocking() + fun `GIVEN 50 killed tabs WHEN another killed tab is reloaded THEN oldest tab is removed and reloaded tab is recorded`() = + runTest { + val oldestTabId = "tab-id-0" + val newTabId = "new-tab-id" + + // Fill recentlyKilledTabs with 50 entries and verify max limit is reached + repeat(50) { i -> + val tabId = "tab-id-$i" + store.dispatch( + TabListAction.AddTabAction( + createTab( + id = tabId, + url = "https://example.com/$i", + ), + ), + ).joinBlocking() + store.dispatch(EngineAction.KillEngineSessionAction(tabId)).joinBlocking() + } + assertTrue(store.state.recentlyKilledTabs.contains(oldestTabId)) + assertEquals(50, store.state.recentlyKilledTabs.size) + + // Kill one more tab and verify oldest tab is removed + store.dispatch( + TabListAction.AddTabAction( + createTab( + id = newTabId, + url = "https://example.com/$newTabId", + ), + ), + ).joinBlocking() + store.dispatch(EngineAction.KillEngineSessionAction(newTabId)).joinBlocking() + assertFalse(store.state.recentlyKilledTabs.contains(oldestTabId)) + assertTrue(store.state.recentlyKilledTabs.contains(newTabId)) + assertEquals(50, store.state.recentlyKilledTabs.size) + + // Verify the reload of the newest tab was recorded + val recordedEventsBefore = EngineMetrics.reloaded.testGetValue()?.size ?: 0 + store.dispatch(EngineAction.CreateEngineSessionAction(newTabId)).joinBlocking() + ShadowLooper.idleMainLooper() + val recordedEventsAfter = EngineMetrics.reloaded.testGetValue() + assertNotNull(recordedEventsAfter) + assertEquals(recordedEventsBefore + 1, recordedEventsAfter!!.size) } - assertTrue(store.state.recentlyKilledTabs.contains(oldestTabId)) - assertEquals(50, store.state.recentlyKilledTabs.size) - - // Kill one more tab and verify oldest tab is removed - store.dispatch(TabListAction.AddTabAction(createTab(id = newTabId, url = "https://example.com/$newTabId"))).joinBlocking() - store.dispatch(EngineAction.KillEngineSessionAction(newTabId)).joinBlocking() - assertFalse(store.state.recentlyKilledTabs.contains(oldestTabId)) - assertTrue(store.state.recentlyKilledTabs.contains(newTabId)) - assertEquals(50, store.state.recentlyKilledTabs.size) - - // Verify the reload of the newest tab was recorded - val recordedEventsBefore = EngineMetrics.reloaded.testGetValue()?.size ?: 0 - store.dispatch(EngineAction.CreateEngineSessionAction(newTabId)).joinBlocking() - ShadowLooper.idleMainLooper() - val recordedEventsAfter = EngineMetrics.reloaded.testGetValue() - assertNotNull(recordedEventsAfter) - assertEquals(recordedEventsBefore + 1, recordedEventsAfter!!.size) - } @Test - fun `WHEN uri loaded to engine THEN matching event is sent to metrics`() { + fun `WHEN uri loaded to engine THEN matching event is sent to metrics`() = runTest { store.dispatch(EngineAction.LoadUrlAction("", "")).joinBlocking() verify { metrics.track(Event.GrowthData.FirstUriLoadForDay) } } @Test - fun `WHEN EnabledAction is dispatched THEN enable the process spawning`() { + fun `WHEN EnabledAction is dispatched THEN enable the process spawning`() = runTest { assertNull(Addons.extensionsProcessUiRetry.testGetValue()) assertNull(Addons.extensionsProcessUiDisable.testGetValue()) @@ -419,7 +462,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN DisabledAction is dispatched THEN disable the process spawning`() { + fun `WHEN DisabledAction is dispatched THEN disable the process spawning`() = runTest { assertNull(Addons.extensionsProcessUiRetry.testGetValue()) assertNull(Addons.extensionsProcessUiDisable.testGetValue()) @@ -430,7 +473,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN TranslateOfferAction is dispatched THEN update telemetry`() { + fun `WHEN TranslateOfferAction is dispatched THEN update telemetry`() = runTest { assertNull(Translations.offerEvent.testGetValue()) store.dispatch(TranslationsAction.TranslateOfferAction(tabId = "1", true)).joinBlocking() @@ -440,7 +483,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN TranslateExpectedAction is dispatched THEN update telemetry`() { + fun `WHEN TranslateExpectedAction is dispatched THEN update telemetry`() = runTest { assertNull(Translations.offerEvent.testGetValue()) store.dispatch(TranslationsAction.TranslateExpectedAction(tabId = "1")).joinBlocking() @@ -450,7 +493,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN TranslateAction is dispatched THEN update telemetry`() { + fun `WHEN TranslateAction is dispatched THEN update telemetry`() = runTest { assertNull(Translations.translateRequested.testGetValue()) store.dispatch( @@ -468,7 +511,7 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN TranslateSuccessAction is dispatched THEN update telemetry`() { + fun `WHEN TranslateSuccessAction is dispatched THEN update telemetry`() = runTest { assertNull(Translations.translateSuccess.testGetValue()) // Shouldn't record other operations @@ -493,59 +536,65 @@ class TelemetryMiddlewareTest { } @Test - fun `WHEN TranslateExceptionAction for Translate operation is dispatched THEN update telemetry`() { - assertNull(Translations.translateFailed.testGetValue()) - - // Shouldn't record other operations - store.dispatch( - TranslationsAction.TranslateExceptionAction( - tabId = "1", - operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES, - translationError = TranslationError.UnknownError(IllegalStateException()), - ), - ).joinBlocking() - assertNull(Translations.translateFailed.testGetValue()) - - // Should record translate operations - store.dispatch( - TranslationsAction.TranslateExceptionAction( - tabId = "1", - operation = TranslationOperation.TRANSLATE, - translationError = TranslationError.CouldNotTranslateError(null), - ), - ).joinBlocking() + fun `WHEN TranslateExceptionAction for Translate operation is dispatched THEN update telemetry`() = + runTest { + assertNull(Translations.translateFailed.testGetValue()) + + // Shouldn't record other operations + store.dispatch( + TranslationsAction.TranslateExceptionAction( + tabId = "1", + operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES, + translationError = TranslationError.UnknownError(IllegalStateException()), + ), + ).joinBlocking() + assertNull(Translations.translateFailed.testGetValue()) + + // Should record translate operations + store.dispatch( + TranslationsAction.TranslateExceptionAction( + tabId = "1", + operation = TranslationOperation.TRANSLATE, + translationError = TranslationError.CouldNotTranslateError(null), + ), + ).joinBlocking() - val telemetry = Translations.translateFailed.testGetValue()?.firstOrNull() - assertEquals(TranslationError.CouldNotTranslateError(cause = null).errorName, telemetry?.extra?.get("error")) - } + val telemetry = Translations.translateFailed.testGetValue()?.firstOrNull() + assertEquals( + TranslationError.CouldNotTranslateError(cause = null).errorName, + telemetry?.extra?.get("error"), + ) + } @Test - fun `WHEN SetEngineSupportedAction is dispatched AND supported THEN update telemetry`() { - assertNull(Translations.engineSupported.testGetValue()) + fun `WHEN SetEngineSupportedAction is dispatched AND supported THEN update telemetry`() = + runTest { + assertNull(Translations.engineSupported.testGetValue()) - store.dispatch( - TranslationsAction.SetEngineSupportedAction( - isEngineSupported = true, - ), - ).joinBlocking() + store.dispatch( + TranslationsAction.SetEngineSupportedAction( + isEngineSupported = true, + ), + ).joinBlocking() - val telemetry = Translations.engineSupported.testGetValue()?.firstOrNull() - assertEquals("supported", telemetry?.extra?.get("support")) - } + val telemetry = Translations.engineSupported.testGetValue()?.firstOrNull() + assertEquals("supported", telemetry?.extra?.get("support")) + } @Test - fun `WHEN SetEngineSupportedAction is dispatched AND unsupported THEN update telemetry`() { - assertNull(Translations.engineSupported.testGetValue()) + fun `WHEN SetEngineSupportedAction is dispatched AND unsupported THEN update telemetry`() = + runTest { + assertNull(Translations.engineSupported.testGetValue()) - store.dispatch( - TranslationsAction.SetEngineSupportedAction( - isEngineSupported = false, - ), - ).joinBlocking() + store.dispatch( + TranslationsAction.SetEngineSupportedAction( + isEngineSupported = false, + ), + ).joinBlocking() - val telemetry = Translations.engineSupported.testGetValue()?.firstOrNull() - assertEquals("unsupported", telemetry?.extra?.get("support")) - } + val telemetry = Translations.engineSupported.testGetValue()?.firstOrNull() + assertEquals("unsupported", telemetry?.extra?.get("support")) + } } internal class FakeClock : Clock.Delegate {