commit 3bc61ed0f6bb0c0cff1b5915d85841eff96e0fac parent fb88a0efb9dd502485eb9db710e3d6e4ac0f21d0 Author: pollymce <pmceldowney@mozilla.com> Date: Mon, 10 Nov 2025 15:03:13 +0000 Bug 1980348 - remove thread marshalling from Stores. r=android-reviewers,matt-tighe Second attempt with a smaller patch size. (see patch https://phabricator.services.mozilla.com/D263733 for first attempt) [Doc to explain the changes here](https://docs.google.com/document/d/1v1cmbK1RxDDgZbXHkt2LKBQAijXg7hpo0_NsP1lZ6go/edit?usp=sharing) Changes that have been made: - remove threading code from Store - fix tests than rely on multithreading or throw exceptions on background threads - For now Store.dispatch() returns a dummy Job. This means the `Store` api, which is used in a lot of tests, doesn't need to be changed as part of this patch. This is temporary and will be cleaned up as soon as we are happy with this change - intention is to have the `dispatch()` function not return and clean up all the `join()`/`joinBlocking()` thread wrangling in unit tests. Differential Revision: https://phabricator.services.mozilla.com/D268398 Diffstat:
43 files changed, 383 insertions(+), 475 deletions(-)
diff --git a/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/store/BrowserStore.kt b/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/store/BrowserStore.kt @@ -27,7 +27,6 @@ class BrowserStore( initialState, BrowserStateReducer::reduce, middleware, - "BrowserStore", ) { init { initialState.selectedTabId?.let { diff --git a/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/PdfStateMiddlewareTest.kt b/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/PdfStateMiddlewareTest.kt @@ -54,7 +54,7 @@ class PdfStateMiddlewareTest { ) store.dispatch(ContentAction.UpdateProgressAction(NORMAL_TAB_ID, 100)).join() - store.waitUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware + testScheduler.advanceUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware assertTrue(captureActionsMiddleware.findFirstAction(ContentAction.EnteredPdfViewer::class).tabId == NORMAL_TAB_ID) } @@ -67,7 +67,7 @@ class PdfStateMiddlewareTest { ) store.dispatch(ContentAction.UpdateProgressAction(CUSTOM_TAB_ID, 100)).join() - store.waitUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware + testScheduler.advanceUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware assertTrue(captureActionsMiddleware.findFirstAction(ContentAction.EnteredPdfViewer::class).tabId == CUSTOM_TAB_ID) } @@ -83,7 +83,7 @@ class PdfStateMiddlewareTest { ) store.dispatch(ContentAction.UpdateProgressAction(NORMAL_TAB_ID, 100)).join() - store.waitUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware + testScheduler.advanceUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware assertTrue(captureActionsMiddleware.findFirstAction(ContentAction.ExitedPdfViewer::class).tabId == NORMAL_TAB_ID) } @@ -99,7 +99,7 @@ class PdfStateMiddlewareTest { ) store.dispatch(ContentAction.UpdateProgressAction(CUSTOM_TAB_ID, 100)).join() - store.waitUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware + testScheduler.advanceUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware assertTrue(captureActionsMiddleware.findFirstAction(ContentAction.ExitedPdfViewer::class).tabId == CUSTOM_TAB_ID) } @@ -127,7 +127,7 @@ class PdfStateMiddlewareTest { ) store.dispatch(ContentAction.UpdateProgressAction(NORMAL_TAB_ID, 100)).join() - store.waitUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware + testScheduler.advanceUntilIdle() // wait for the actions dispatched from PdfStateMiddleware to be handled in CaptureActionsMiddleware assertTrue(captureActionsMiddleware.findFirstAction(ContentAction.ExitedPdfViewer::class).tabId == NORMAL_TAB_ID) } diff --git a/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt b/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt @@ -117,6 +117,7 @@ class TranslationsMiddlewareTest { doReturn(mockSessionState).`when`(tab).translationsState doReturn(mockBrowserState).`when`(state).translationEngine + whenever(store.state).thenReturn(state) } @Test diff --git a/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/store/BrowserStoreExceptionTest.kt b/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/store/BrowserStoreExceptionTest.kt @@ -12,13 +12,9 @@ import mozilla.components.browser.state.state.TabGroup import mozilla.components.browser.state.state.TabPartition import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.state.recover.toRecoverableTab -import mozilla.components.lib.state.StoreException import mozilla.components.support.test.ext.joinBlocking -import org.junit.Assert.fail import org.junit.Test import org.junit.runner.RunWith -import org.robolectric.shadows.ShadowLooper -import java.lang.IllegalArgumentException // These tests are in a separate class because they needs to run with // Robolectric (different runner, slower) while all other tests only @@ -26,176 +22,149 @@ import java.lang.IllegalArgumentException @RunWith(AndroidJUnit4::class) class BrowserStoreExceptionTest { - @Test(expected = java.lang.IllegalArgumentException::class) + @Test(expected = IllegalArgumentException::class) fun `AddTabAction - Exception is thrown if parent doesn't exist`() { - unwrapStoreExceptionAndRethrow { - val store = BrowserStore() - val parent = createTab("https://www.mozilla.org") - val child = createTab("https://www.firefox.com", parent = parent) + val store = BrowserStore() + val parent = createTab("https://www.mozilla.org") + val child = createTab("https://www.firefox.com", parent = parent) - store.dispatch(TabListAction.AddTabAction(child)).joinBlocking() - } + store.dispatch(TabListAction.AddTabAction(child)).joinBlocking() } - @Test(expected = java.lang.IllegalArgumentException::class) + @Test(expected = IllegalArgumentException::class) fun `AddTabAction - Exception is thrown if tab already exists`() { - unwrapStoreExceptionAndRethrow { - val store = BrowserStore() - val tab1 = createTab("https://www.mozilla.org") - store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking() - store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking() - } + val store = BrowserStore() + val tab1 = createTab("https://www.mozilla.org") + store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking() + store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking() } - @Test(expected = java.lang.IllegalArgumentException::class) + @Test(expected = IllegalArgumentException::class) fun `RestoreTabAction - Exception is thrown if tab already exists`() { - unwrapStoreExceptionAndRethrow { - val store = BrowserStore() - val tab1 = createTab("https://www.mozilla.org") - store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking() - - store.dispatch(TabListAction.RestoreAction(listOf(tab1.toRecoverableTab()), restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING)).joinBlocking() - } + val store = BrowserStore() + val tab1 = createTab("https://www.mozilla.org") + store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking() + + store.dispatch( + TabListAction.RestoreAction( + listOf(tab1.toRecoverableTab()), + restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING, + ), + ).joinBlocking() } @Test(expected = IllegalArgumentException::class) fun `AddMultipleTabsAction - Exception is thrown in tab with id already exists`() { - unwrapStoreExceptionAndRethrow { - val store = BrowserStore( - BrowserState( - tabs = listOf( - createTab(id = "a", url = "https://www.mozilla.org"), - ), + val store = BrowserStore( + BrowserState( + tabs = listOf( + createTab(id = "a", url = "https://www.mozilla.org"), ), - ) + ), + ) - store.dispatch( - TabListAction.AddMultipleTabsAction( - tabs = listOf( - createTab(id = "a", url = "https://www.example.org"), - ), + store.dispatch( + TabListAction.AddMultipleTabsAction( + tabs = listOf( + createTab(id = "a", url = "https://www.example.org"), ), - ).joinBlocking() - } + ), + ).joinBlocking() } @Test(expected = IllegalArgumentException::class) fun `AddMultipleTabsAction - Exception is thrown if parent id is set`() { - unwrapStoreExceptionAndRethrow { - val store = BrowserStore() - - val tab1 = createTab( - id = "a", - url = "https://www.mozilla.org", - ) - - val tab2 = createTab( - id = "b", - url = "https://www.firefox.com", - private = true, - parent = tab1, - ) - - store.dispatch( - TabListAction.AddMultipleTabsAction( - tabs = listOf(tab1, tab2), - ), - ).joinBlocking() - } + val store = BrowserStore() + + val tab1 = createTab( + id = "a", + url = "https://www.mozilla.org", + ) + + val tab2 = createTab( + id = "b", + url = "https://www.firefox.com", + private = true, + parent = tab1, + ) + + store.dispatch( + TabListAction.AddMultipleTabsAction( + tabs = listOf(tab1, tab2), + ), + ).joinBlocking() } - @Test(expected = java.lang.IllegalArgumentException::class) + @Test(expected = IllegalArgumentException::class) fun `AddTabGroupAction - Exception is thrown when group already exists`() { - unwrapStoreExceptionAndRethrow { - val partitionId = "testFeaturePartition" - val testGroup = TabGroup("test") - val store = BrowserStore( - BrowserState( - tabPartitions = mapOf(partitionId to TabPartition(partitionId, tabGroups = listOf(testGroup))), - ), - ) - - store.dispatch( - TabGroupAction.AddTabGroupAction( - partition = partitionId, - group = testGroup, + val partitionId = "testFeaturePartition" + val testGroup = TabGroup("test") + val store = BrowserStore( + BrowserState( + tabPartitions = mapOf( + partitionId to TabPartition( + partitionId, + tabGroups = listOf(testGroup), + ), ), - ).joinBlocking() - } + ), + ) + + store.dispatch( + TabGroupAction.AddTabGroupAction( + partition = partitionId, + group = testGroup, + ), + ).joinBlocking() } - @Test(expected = java.lang.IllegalArgumentException::class) + @Test(expected = IllegalArgumentException::class) fun `AddTabGroupAction - Asserts that tabs exist`() { - unwrapStoreExceptionAndRethrow { - val store = BrowserStore() - - val partition = "testFeaturePartition" - val testGroup = TabGroup("test", tabIds = listOf("invalid")) - store.dispatch( - TabGroupAction.AddTabGroupAction( - partition = partition, - group = testGroup, - ), - ).joinBlocking() - } + val store = BrowserStore() + + val partition = "testFeaturePartition" + val testGroup = TabGroup("test", tabIds = listOf("invalid")) + store.dispatch( + TabGroupAction.AddTabGroupAction( + partition = partition, + group = testGroup, + ), + ).joinBlocking() } - @Test(expected = java.lang.IllegalArgumentException::class) + @Test(expected = IllegalArgumentException::class) fun `AddTabAction - Asserts that tab exists when adding to group`() { - unwrapStoreExceptionAndRethrow { - val tabGroup = TabGroup("test1", tabIds = emptyList()) - val tabPartition = TabPartition("testFeaturePartition", tabGroups = listOf(tabGroup)) - - val store = BrowserStore( - BrowserState( - tabs = listOf(), - tabPartitions = mapOf("testFeaturePartition" to tabPartition), - ), - ) - - val tab = createTab(id = "tab1", url = "https://firefox.com") - store.dispatch(TabGroupAction.AddTabAction(tabPartition.id, tabGroup.id, tab.id)).joinBlocking() - } + val tabGroup = TabGroup("test1", tabIds = emptyList()) + val tabPartition = TabPartition("testFeaturePartition", tabGroups = listOf(tabGroup)) + + val store = BrowserStore( + BrowserState( + tabs = listOf(), + tabPartitions = mapOf("testFeaturePartition" to tabPartition), + ), + ) + + val tab = createTab(id = "tab1", url = "https://firefox.com") + store.dispatch(TabGroupAction.AddTabAction(tabPartition.id, tabGroup.id, tab.id)) + .joinBlocking() } - @Test(expected = java.lang.IllegalArgumentException::class) + @Test(expected = IllegalArgumentException::class) fun `AddTabsAction - Asserts that tabs exist when adding to group`() { - unwrapStoreExceptionAndRethrow { - val tabGroup = TabGroup("test1", tabIds = emptyList()) - val tabPartition = TabPartition("testFeaturePartition", tabGroups = listOf(tabGroup)) - val tab1 = createTab(id = "tab1", url = "https://firefox.com") - val tab2 = createTab(id = "tab2", url = "https://mozilla.org") - - val store = BrowserStore( - BrowserState( - tabs = listOf(tab1), - tabPartitions = mapOf("testFeaturePartition" to tabPartition), - ), - ) - - store.dispatch( - TabGroupAction.AddTabsAction(tabPartition.id, tabGroup.id, listOf(tab1.id, tab2.id)), - ).joinBlocking() - } - } - - private fun unwrapStoreExceptionAndRethrow(block: () -> Unit) { - try { - block() - - // Wait for the main looper to process the re-thrown exception. - ShadowLooper.idleMainLooper() - - fail("Did not throw StoreException") - } catch (e: StoreException) { - val cause = e.cause - if (cause != null) { - throw cause - } - } catch (e: Throwable) { - fail("Did throw a different exception $e") - } - - fail("Did not throw StoreException with wrapped exception") + val tabGroup = TabGroup("test1", tabIds = emptyList()) + val tabPartition = TabPartition("testFeaturePartition", tabGroups = listOf(tabGroup)) + val tab1 = createTab(id = "tab1", url = "https://firefox.com") + val tab2 = createTab(id = "tab2", url = "https://mozilla.org") + + val store = BrowserStore( + BrowserState( + tabs = listOf(tab1), + tabPartitions = mapOf("testFeaturePartition" to tabPartition), + ), + ) + + store.dispatch( + TabGroupAction.AddTabsAction(tabPartition.id, tabGroup.id, listOf(tab1.id, tab2.id)), + ).joinBlocking() } } diff --git a/mobile/android/android-components/components/browser/thumbnails/src/test/java/mozilla/components/browser/thumbnails/BrowserThumbnailsTest.kt b/mobile/android/android-components/components/browser/thumbnails/src/test/java/mozilla/components/browser/thumbnails/BrowserThumbnailsTest.kt @@ -48,6 +48,7 @@ class BrowserThumbnailsTest { ), selectedTabId = tabId, ), + middleware = listOf(ThumbnailsMiddleware(mock())), ), ) engineView = mock() diff --git a/mobile/android/android-components/components/feature/contextmenu/src/test/java/mozilla/components/feature/contextmenu/ContextMenuCandidateTest.kt b/mobile/android/android-components/components/feature/contextmenu/src/test/java/mozilla/components/feature/contextmenu/ContextMenuCandidateTest.kt @@ -155,6 +155,9 @@ class ContextMenuCandidateTest { createTab("https://www.mozilla.org", contextId = "1"), ), ), + middleware = EngineMiddleware.create( + engine = mock(), + ), ) val tabsUseCases = TabsUseCases(store) @@ -185,6 +188,9 @@ class ContextMenuCandidateTest { createTab("https://www.mozilla.org"), ), ), + middleware = EngineMiddleware.create( + engine = mock(), + ), ) val tabsUseCases = TabsUseCases(store) @@ -255,6 +261,9 @@ class ContextMenuCandidateTest { ), selectedTabId = "mozilla", ), + middleware = EngineMiddleware.create( + engine = mock(), + ), ) val tabsUseCases = TabsUseCases(store) @@ -282,7 +291,7 @@ class ContextMenuCandidateTest { fun `Open Link in New Tab with text fragment`() { val middleware = CaptureActionsMiddleware<BrowserState, BrowserAction>() val store = BrowserStore( - middleware = listOf(middleware), + middleware = listOf(middleware) + EngineMiddleware.create(engine = mock()), initialState = BrowserState( tabs = listOf( createTab("https://www.mozilla.org", id = "mozilla"), @@ -415,6 +424,9 @@ class ContextMenuCandidateTest { ), selectedTabId = "mozilla", ), + middleware = EngineMiddleware.create( + engine = mock(), + ), ) val tabsUseCases = TabsUseCases(store) @@ -486,6 +498,9 @@ class ContextMenuCandidateTest { ), selectedTabId = "mozilla", ), + middleware = EngineMiddleware.create( + engine = mock(), + ), ) val tabsUseCases = TabsUseCases(store) @@ -633,6 +648,9 @@ class ContextMenuCandidateTest { ), selectedTabId = "mozilla", ), + middleware = EngineMiddleware.create( + engine = mock(), + ), ) val tabsUseCases = TabsUseCases(store) @@ -667,6 +685,9 @@ class ContextMenuCandidateTest { ), selectedTabId = "mozilla", ), + middleware = EngineMiddleware.create( + engine = mock(), + ), ) val tabsUseCases = TabsUseCases(store) diff --git a/mobile/android/android-components/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/store/CustomTabsServiceStore.kt b/mobile/android/android-components/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/store/CustomTabsServiceStore.kt @@ -11,5 +11,4 @@ class CustomTabsServiceStore( ) : Store<CustomTabsServiceState, CustomTabsAction>( initialState, CustomTabsServiceStateReducer::reduce, - threadNamePrefix = "CustomTabsService", ) diff --git a/mobile/android/android-components/components/feature/customtabs/src/test/java/mozilla/components/feature/customtabs/CustomTabIntentProcessorTest.kt b/mobile/android/android-components/components/feature/customtabs/src/test/java/mozilla/components/feature/customtabs/CustomTabIntentProcessorTest.kt @@ -12,6 +12,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.components.browser.state.action.BrowserAction import mozilla.components.browser.state.action.CustomTabListAction import mozilla.components.browser.state.action.EngineAction +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.selector.findCustomTab import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.SessionState.Source @@ -41,7 +42,11 @@ class CustomTabIntentProcessorTest { @Test fun processCustomTabIntentWithDefaultHandlers() { val middleware = CaptureActionsMiddleware<BrowserState, BrowserAction>() - val store = BrowserStore(middleware = listOf(middleware)) + val store = BrowserStore( + middleware = listOf(middleware) + EngineMiddleware.create( + engine = mock(), + ), + ) val useCases = SessionUseCases(store) val customTabsUseCases = CustomTabsUseCases(store, useCases.loadUrl) @@ -83,7 +88,11 @@ class CustomTabIntentProcessorTest { @Test fun processCustomTabIntentWithAdditionalHeaders() { val middleware = CaptureActionsMiddleware<BrowserState, BrowserAction>() - val store = BrowserStore(middleware = listOf(middleware)) + val store = BrowserStore( + middleware = listOf(middleware) + EngineMiddleware.create( + engine = mock(), + ), + ) val useCases = SessionUseCases(store) val customTabsUseCases = CustomTabsUseCases(store, useCases.loadUrl) @@ -132,7 +141,11 @@ class CustomTabIntentProcessorTest { @Test fun processPrivateCustomTabIntentWithDefaultHandlers() { val middleware = CaptureActionsMiddleware<BrowserState, BrowserAction>() - val store = BrowserStore(middleware = listOf(middleware)) + val store = BrowserStore( + middleware = listOf(middleware) + EngineMiddleware.create( + engine = mock(), + ), + ) val useCases = SessionUseCases(store) val customTabsUseCases = CustomTabsUseCases(store, useCases.loadUrl) diff --git a/mobile/android/android-components/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt b/mobile/android/android-components/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt @@ -20,6 +20,7 @@ import androidx.fragment.app.FragmentTransaction import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.TabListAction +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ContentState import mozilla.components.browser.state.state.TabSessionState @@ -123,6 +124,7 @@ class PromptFeatureTest { ), selectedTabId = tabId, ), + middleware = EngineMiddleware.create(mock()), ) loginPicker = mock() creditCardPicker = mock() diff --git a/mobile/android/android-components/components/feature/readerview/src/test/java/mozilla/components/feature/readerview/ReaderViewFeatureTest.kt b/mobile/android/android-components/components/feature/readerview/src/test/java/mozilla/components/feature/readerview/ReaderViewFeatureTest.kt @@ -11,6 +11,7 @@ import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.EngineAction import mozilla.components.browser.state.action.ReaderAction import mozilla.components.browser.state.action.TabListAction +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ReaderState import mozilla.components.browser.state.state.TabSessionState @@ -275,6 +276,7 @@ class ReaderViewFeatureTest { tabs = listOf(tab), selectedTabId = tab.id, ), + middleware = EngineMiddleware.create(engine), ), ) diff --git a/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/SearchUseCasesTest.kt b/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/SearchUseCasesTest.kt @@ -9,6 +9,7 @@ import mozilla.components.browser.state.action.BrowserAction import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.EngineAction import mozilla.components.browser.state.action.TabListAction +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.search.RegionState import mozilla.components.browser.state.search.SearchEngine import mozilla.components.browser.state.state.BrowserState @@ -71,7 +72,9 @@ class SearchUseCasesTest { regionSearchEngines = listOf(searchEngine), ), ), - middleware = listOf(middleware), + middleware = listOf(middleware) + EngineMiddleware.create( + engine = mock(), + ), ) useCases = SearchUseCases( diff --git a/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/middleware/AdsTelemetryMiddlewareTest.kt b/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/middleware/AdsTelemetryMiddlewareTest.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.search.middleware +import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.TabListAction import mozilla.components.browser.state.state.BrowserState @@ -20,8 +21,10 @@ import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Before import org.junit.Test +import org.junit.runner.RunWith import org.mockito.Mockito.verify +@RunWith(AndroidJUnit4::class) class AdsTelemetryMiddlewareTest { val sessionId = "session" lateinit var adsMiddleware: AdsTelemetryMiddleware diff --git a/mobile/android/android-components/components/feature/sitepermissions/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsFeatureTest.kt b/mobile/android/android-components/components/feature/sitepermissions/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsFeatureTest.kt @@ -25,12 +25,14 @@ import mozilla.components.browser.state.action.ContentAction.UpdatePermissionHig import mozilla.components.browser.state.action.ContentAction.UpdatePermissionHighlightsStateAction.MicrophoneChangedAction import mozilla.components.browser.state.action.ContentAction.UpdatePermissionHighlightsStateAction.NotificationChangedAction import mozilla.components.browser.state.action.ContentAction.UpdatePermissionHighlightsStateAction.PersistentStorageChangedAction +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ContentState import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.permission.Permission import mozilla.components.concept.engine.permission.Permission.AppAudio import mozilla.components.concept.engine.permission.Permission.ContentAudioCapture @@ -128,7 +130,19 @@ class SitePermissionsFeatureTest { url = "https://www.mozilla.org", id = SESSION_ID, ) - mockStore = spy(BrowserStore(initialState = BrowserState(tabs = listOf(selectedTab), selectedTabId = selectedTab.id))) + val mockEngine = mock<Engine>() + whenever(mockEngine.createSession()).thenReturn(mock()) + mockStore = spy( + BrowserStore( + initialState = BrowserState( + tabs = listOf(selectedTab), + selectedTabId = selectedTab.id, + ), + middleware = EngineMiddleware.create( + engine = mockEngine, + ), + ), + ) sitePermissionFeature = spy( SitePermissionsFeature( context = testContext, diff --git a/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/Store.kt b/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/Store.kt @@ -6,15 +6,11 @@ package mozilla.components.lib.state import androidx.annotation.CheckResult import androidx.annotation.VisibleForTesting -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.launch -import mozilla.components.lib.state.internal.DefaultStoreDispatcher +import kotlinx.coroutines.Job import mozilla.components.lib.state.internal.ReducerChainBuilder -import mozilla.components.lib.state.internal.StoreDispatcher import java.lang.ref.WeakReference import java.util.Collections import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.Executors /** * A generic store holding an immutable [State]. @@ -26,34 +22,12 @@ import java.util.concurrent.Executors * @param reducer A function that gets the current [State] and [Action] passed in and will return a new [State]. * @param middleware Optional list of [Middleware] sitting between the [Store] and the [Reducer]. */ -open class Store<S : State, A : Action> internal constructor( +open class Store<S : State, A : Action>( initialState: S, reducer: Reducer<S, A>, - middleware: List<Middleware<S, A>>, - dispatcher: StoreDispatcher, + middleware: List<Middleware<S, A>> = emptyList(), ) { - - /** - * @param initialState The initial state until a dispatched [Action] creates a new state. - * @param reducer A function that gets the current [State] and [Action] passed in and will return a new [State]. - * @param middleware Optional list of [Middleware] sitting between the [Store] and the [Reducer]. - * @param threadNamePrefix Optional prefix with which to name threads for the [Store]. If not provided, - * the naming scheme will be deferred to [Executors.defaultThreadFactory] - */ - constructor( - initialState: S, - reducer: Reducer<S, A>, - middleware: List<Middleware<S, A>> = emptyList(), - threadNamePrefix: String? = null, - ) : this( - initialState = initialState, - reducer = reducer, - middleware = middleware, - dispatcher = DefaultStoreDispatcher(threadNamePrefix), - ) - - private val reducerChainBuilder = ReducerChainBuilder(dispatcher, reducer, middleware) - private val scope = CoroutineScope(dispatcher.coroutineContext) + private val reducerChainBuilder = ReducerChainBuilder(reducer, middleware) @VisibleForTesting internal val subscriptions = Collections.newSetFromMap(ConcurrentHashMap<Subscription<S, A>, Boolean>()) @@ -90,11 +64,23 @@ open class Store<S : State, A : Action> internal constructor( /** * Dispatch an [Action] to the store in order to trigger a [State] change. + * This function may be invoked on any thread. + * Invocations are serialized by synchronizing on `this@Store`, + * preventing concurrent modification of the underlying store. + * Long running reducers and/or middlewares can and will impact all consumers. */ - fun dispatch(action: A) = scope.launch { + fun dispatch(action: A): Job { synchronized(this@Store) { reducerChainBuilder.get(this@Store).invoke(action) } + // see https://bugzilla.mozilla.org/show_bug.cgi?id=1980348 + // previously, we launched a new coroutine here. + // this `Job()` is a dummy implementation for now to avoiding having to change the api. + // the aspiration is to remove this, simplify the api and delete a load of joining code. + val job = Job().also { + it.complete() + } + return job } /** diff --git a/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/StoreException.kt b/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/StoreException.kt @@ -1,11 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package mozilla.components.lib.state - -/** - * Exception for otherwise unhandled errors caught while reducing state or - * while managing/notifying observers. - */ -class StoreException(msg: String, val e: Throwable? = null) : Exception(msg, e) diff --git a/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/UiStore.kt b/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/UiStore.kt @@ -5,7 +5,6 @@ package mozilla.components.lib.state import androidx.annotation.MainThread -import mozilla.components.lib.state.internal.UiStoreDispatcher /** * A generic store holding an immutable [State] that will always run on the Main thread. @@ -26,5 +25,4 @@ open class UiStore<S : State, A : Action>( initialState, reducer, middleware, - UiStoreDispatcher(), ) diff --git a/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/DefaultStoreDispatcher.kt b/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/DefaultStoreDispatcher.kt @@ -1,69 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package mozilla.components.lib.state.internal - -import android.os.Handler -import android.os.Looper -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.CoroutineExceptionHandler -import kotlinx.coroutines.asCoroutineDispatcher -import kotlinx.coroutines.cancel -import mozilla.components.lib.state.Action -import mozilla.components.lib.state.Store -import mozilla.components.lib.state.StoreException -import mozilla.components.support.utils.ext.threadIdCompat -import java.util.concurrent.Executors -import java.util.concurrent.ThreadFactory - -/** - * The default [StoreDispatcher] used in a [Store]. It uses a single thread for dispatching and - * processing [Action]s. - */ -internal class DefaultStoreDispatcher( - threadNamePrefix: String? = null, -) : StoreDispatcher { - private val storeThreadFactory = StoreThreadFactory(threadNamePrefix) - private val threadFactory: ThreadFactory = storeThreadFactory - private val exceptionHandler = CoroutineExceptionHandler { coroutineContext, throwable -> - // We want exceptions in the reducer to crash the app and not get silently ignored. Therefore we rethrow the - // exception on the main thread. - Handler(Looper.getMainLooper()).postAtFrontOfQueue { - throw StoreException("Exception while reducing state", throwable) - } - - // Once an exception happened we do not want to accept any further actions. So let's cancel the scope which - // will cancel all jobs and not accept any new ones. - coroutineContext.cancel() - } - - private val dispatcher: CoroutineDispatcher by lazy { - Executors.newSingleThreadExecutor( - threadFactory, - ).asCoroutineDispatcher() - } - - /** - * See [StoreDispatcher.coroutineContext]. - */ - override val coroutineContext = dispatcher + exceptionHandler - - /** - * See [StoreDispatcher.assertOnThread]. - */ - override fun assertOnThread() { - val currentThread = Thread.currentThread() - val currentThreadId = currentThread.threadIdCompat() - val expectedThreadId = storeThreadFactory.threadId - - if (currentThreadId == expectedThreadId) { - return - } - - throw IllegalThreadStateException( - "Expected `store` thread, but running on thread `${currentThread.name}`. " + - "Leaked MiddlewareContext or did you mean to use `MiddlewareContext.store.dispatch`?", - ) - } -} diff --git a/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/ReducerChainBuilder.kt b/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/ReducerChainBuilder.kt @@ -16,7 +16,6 @@ import mozilla.components.lib.state.Store * [reducer]. */ internal class ReducerChainBuilder<S : State, A : Action>( - private val storeDispatcher: StoreDispatcher, private val reducer: Reducer<S, A>, private val middleware: List<Middleware<S, A>>, ) { @@ -52,12 +51,7 @@ internal class ReducerChainBuilder<S : State, A : Action>( store.transitionTo(state) } - val threadCheck: Middleware<S, A> = { _, next, action -> - storeDispatcher.assertOnThread() - next(action) - } - - (middleware.reversed() + threadCheck).forEach { middleware -> + middleware.reversed().forEach { middleware -> val next = chain chain = { action -> middleware(context, next, action) } } diff --git a/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/StoreDispatcher.kt b/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/StoreDispatcher.kt @@ -1,27 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package mozilla.components.lib.state.internal - -import kotlinx.coroutines.CoroutineScope -import mozilla.components.lib.state.Store -import kotlin.coroutines.CoroutineContext - -/** - * The dispatcher used by a [Store]. - */ -internal interface StoreDispatcher { - - /** - * A [CoroutineContext] that is used within this dispatcher. Typical implementations create a - * [CoroutineScope] using this context. - */ - val coroutineContext: CoroutineContext - - /** - * Asserts that work done is always on the [StoreDispatcher] thread. - */ - @Throws(IllegalThreadStateException::class) - fun assertOnThread() -} diff --git a/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/StoreThreadFactory.kt b/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/StoreThreadFactory.kt @@ -1,43 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package mozilla.components.lib.state.internal - -import mozilla.components.lib.state.Store -import mozilla.components.support.base.utils.NamedThreadFactory -import mozilla.components.support.utils.ext.threadIdCompat -import java.util.concurrent.Executors -import java.util.concurrent.ThreadFactory - -/** - * Custom [ThreadFactory] implementation wrapping [Executors.defaultThreadFactory]/[NamedThreadFactory] - * that allows asserting whether a caller is on the created thread. - * - * For usage with [Executors.newSingleThreadExecutor]: Only the last created thread is kept and - * compared when [StoreDispatcher.assertOnThread] is called with [threadId]. - * - * @param threadNamePrefix Optional prefix with which to name threads for the [Store]. If not provided, - * the naming scheme will be deferred to [Executors.defaultThreadFactory] - */ -internal class StoreThreadFactory( - threadNamePrefix: String?, -) : ThreadFactory { - @Volatile - private var thread: Thread? = null - - private val actualFactory = if (threadNamePrefix != null) { - NamedThreadFactory(threadNamePrefix) - } else { - Executors.defaultThreadFactory() - } - - val threadId: Long? - get() = thread?.threadIdCompat() - - override fun newThread(r: Runnable): Thread { - return actualFactory.newThread(r).also { - thread = it - } - } -} diff --git a/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/UiStoreDispatcher.kt b/mobile/android/android-components/components/lib/state/src/main/java/mozilla/components/lib/state/internal/UiStoreDispatcher.kt @@ -1,41 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package mozilla.components.lib.state.internal - -import android.os.Looper -import kotlinx.coroutines.Dispatchers -import mozilla.components.lib.state.Action -import mozilla.components.lib.state.UiStore -import mozilla.components.support.utils.ext.threadIdCompat -import kotlin.coroutines.CoroutineContext - -/** - * The default [StoreDispatcher] used in a [UiStore]. It uses the [Dispatchers.Main] thread for - * dispatching and processing [Action]s. - */ -internal class UiStoreDispatcher : StoreDispatcher { - - /** - * See [StoreDispatcher.coroutineContext]. - */ - override val coroutineContext: CoroutineContext = Dispatchers.Main.immediate - - /** - * See [StoreDispatcher.assertOnThread]. - */ - override fun assertOnThread() { - val currentThread = Thread.currentThread() - val expectedThreadId = Looper.getMainLooper().thread.threadIdCompat() - - if (currentThread.threadIdCompat() == expectedThreadId) { - return - } - - throw IllegalThreadStateException( - "Expected Main thread, but running on thread `${currentThread.name}`. " + - "Leaked MiddlewareContext or did you mean to use `MiddlewareContext.store.dispatch`?", - ) - } -} diff --git a/mobile/android/android-components/components/lib/state/src/test/java/mozilla/components/lib/state/StoreExceptionTest.kt b/mobile/android/android-components/components/lib/state/src/test/java/mozilla/components/lib/state/StoreExceptionTest.kt @@ -5,28 +5,23 @@ package mozilla.components.lib.state import androidx.test.ext.junit.runners.AndroidJUnit4 -import mozilla.components.support.test.ext.joinBlocking import org.junit.Assert import org.junit.Test import org.junit.runner.RunWith -import org.robolectric.shadows.ShadowLooper @RunWith(AndroidJUnit4::class) class StoreExceptionTest { // This test is in a separate class because it needs to run with Robolectric (different runner, slower) while all // other tests only need a Java VM (fast). - @Test(expected = StoreException::class) - fun `Exception in reducer will be rethrown on main thread`() { + @Test(expected = IllegalStateException::class) + fun `Exception in reducer will be thrown from store`() { val throwingReducer: (TestState, TestAction) -> TestState = { _, _ -> throw IllegalStateException("Not reducing today") } val store = Store(TestState(counter = 23), throwingReducer) - store.dispatch(TestAction.IncrementAction).joinBlocking() - - // Wait for the main looper to process the re-thrown exception. - ShadowLooper.idleMainLooper() + store.dispatch(TestAction.IncrementAction) Assert.fail() } diff --git a/mobile/android/android-components/components/lib/state/src/test/java/mozilla/components/lib/state/StoreTest.kt b/mobile/android/android-components/components/lib/state/src/test/java/mozilla/components/lib/state/StoreTest.kt @@ -4,13 +4,19 @@ package mozilla.components.lib.state -import mozilla.components.support.test.ext.joinBlocking +import kotlinx.coroutines.Job +import kotlinx.coroutines.asCoroutineDispatcher +import kotlinx.coroutines.delay +import kotlinx.coroutines.joinAll +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Test import java.io.IOException +import java.util.concurrent.Executors class StoreTest { @Test @@ -20,12 +26,12 @@ class StoreTest { ::reducer, ) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(24, store.state.counter) - store.dispatch(TestAction.DecrementAction).joinBlocking() - store.dispatch(TestAction.DecrementAction).joinBlocking() + store.dispatch(TestAction.DecrementAction) + store.dispatch(TestAction.DecrementAction) assertEquals(22, store.state.counter) } @@ -43,7 +49,7 @@ class StoreTest { it.resume() } - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(24, observedValue) } @@ -81,7 +87,7 @@ class StoreTest { assertTrue(stateChangeObserved) stateChangeObserved = false - store.dispatch(TestAction.DoNothingAction).joinBlocking() + store.dispatch(TestAction.DoNothingAction) assertFalse(stateChangeObserved) } @@ -101,17 +107,17 @@ class StoreTest { it.resume() } - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(24, observedValue) - store.dispatch(TestAction.DecrementAction).joinBlocking() + store.dispatch(TestAction.DecrementAction) assertEquals(23, observedValue) subscription.unsubscribe() - store.dispatch(TestAction.DecrementAction).joinBlocking() + store.dispatch(TestAction.DecrementAction) assertEquals(23, observedValue) assertEquals(22, store.state.counter) @@ -144,19 +150,19 @@ class StoreTest { ), ) - store.dispatch(TestAction.DoNothingAction).joinBlocking() + store.dispatch(TestAction.DoNothingAction) assertEquals(2, store.state.counter) - store.dispatch(TestAction.DoNothingAction).joinBlocking() + store.dispatch(TestAction.DoNothingAction) assertEquals(6, store.state.counter) - store.dispatch(TestAction.DoNothingAction).joinBlocking() + store.dispatch(TestAction.DoNothingAction) assertEquals(14, store.state.counter) - store.dispatch(TestAction.DecrementAction).joinBlocking() + store.dispatch(TestAction.DecrementAction) assertEquals(13, store.state.counter) } @@ -173,13 +179,13 @@ class StoreTest { listOf(interceptingMiddleware), ) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(0, store.state.counter) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(0, store.state.counter) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(0, store.state.counter) } @@ -195,13 +201,13 @@ class StoreTest { listOf(rewritingMiddleware), ) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(-1, store.state.counter) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(-2, store.state.counter) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(-3, store.state.counter) } @@ -221,13 +227,13 @@ class StoreTest { listOf(rewritingMiddleware), ) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(-1, store.state.counter) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(-2, store.state.counter) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(-3, store.state.counter) } @@ -248,19 +254,19 @@ class StoreTest { listOf(observingMiddleware), ) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(0, countBefore) assertEquals(1, countAfter) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(1, countBefore) assertEquals(2, countAfter) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertEquals(2, countBefore) assertEquals(3, countAfter) - store.dispatch(TestAction.DecrementAction).joinBlocking() + store.dispatch(TestAction.DecrementAction) assertEquals(3, countBefore) assertEquals(2, countAfter) } @@ -283,9 +289,48 @@ class StoreTest { listOf(catchingMiddleware), ) - store.dispatch(TestAction.IncrementAction).joinBlocking() + store.dispatch(TestAction.IncrementAction) assertNotNull(caughtException) assertTrue(caughtException is IOException) } + + @Test + fun `Dispatching Actions from a different thread from middleware does not create concurrent write problems`() = runTest { + val middlewareJobs = mutableListOf<Job>() + val middlewareDispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher() + val storeDispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher() + val middleware = object : Middleware<TestState, TestAction> { + override fun invoke( + context: MiddlewareContext<TestState, TestAction>, + next: (TestAction) -> Unit, + action: TestAction, + ) { + if (action is TestAction.DecrementAction) { + middlewareJobs += this@runTest.launch(middlewareDispatcher) { + delay(5) + context.store.dispatch(TestAction.IncrementAction) + } + } + next(action) + } + } + val store = Store( + TestState(counter = 23), + ::reducer, + listOf(middleware), + ) + + val storeJobs = mutableListOf<Job>() + for (i in 0..10_000) { + storeJobs += this.launch(storeDispatcher) { store.dispatch(TestAction.DecrementAction) } + } + storeJobs.joinAll() + assertEquals(10_001 * 2, storeJobs.size + middlewareJobs.size) + middlewareJobs.joinAll() + middlewareDispatcher.close() + storeDispatcher.close() + + assertEquals(23, store.state.counter) + } } diff --git a/mobile/android/android-components/components/lib/state/src/test/java/mozilla/components/lib/state/UiStoreTest.kt b/mobile/android/android-components/components/lib/state/src/test/java/mozilla/components/lib/state/UiStoreTest.kt @@ -5,9 +5,6 @@ package mozilla.components.lib.state import androidx.test.ext.junit.runners.AndroidJUnit4 -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch import mozilla.components.support.test.rule.MainCoroutineRule import mozilla.components.support.test.rule.runTestOnMain import org.junit.Assert.assertEquals @@ -102,22 +99,4 @@ class UiStoreTest { assertTrue(caughtException is IOException) } - - @Test(expected = IllegalThreadStateException::class) - fun `Dispatching on a non-UI dispatcher will throw an exception`() = runTestOnMain { - val observingMiddleware: Middleware<TestState, TestAction> = { store, next, action -> - CoroutineScope(Dispatchers.IO).launch { - store.dispatch(TestAction.IncrementAction) - } - - next(action) - } - val store = UiStore( - TestState(counter = 0), - ::reducer, - listOf(observingMiddleware), - ) - - store.dispatch(TestAction.DoNothingAction) - } } diff --git a/mobile/android/android-components/components/support/test-libstate/src/main/java/mozilla/components/support/test/libstate/ext/Store.kt b/mobile/android/android-components/components/support/test-libstate/src/main/java/mozilla/components/support/test/libstate/ext/Store.kt @@ -4,9 +4,6 @@ package mozilla.components.support.test.libstate.ext -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job -import kotlinx.coroutines.runBlocking import mozilla.components.lib.state.Action import mozilla.components.lib.state.State import mozilla.components.lib.state.Store @@ -17,10 +14,6 @@ import mozilla.components.lib.state.Store * state changes. */ fun <S : State, A : Action> Store<S, A>.waitUntilIdle() { - val scopeField = Store::class.java.getDeclaredField("scope") - scopeField.isAccessible = true - val scope = scopeField.get(this) as CoroutineScope - runBlocking { - scope.coroutineContext[Job]?.children?.forEach { it.join() } - } + // see https://bugzilla.mozilla.org/show_bug.cgi?id=1980348 + // this is no longer required and will be deleted later in the stack } diff --git a/mobile/android/android-components/components/support/webextensions/src/test/java/mozilla/components/support/webextensions/WebExtensionSupportTest.kt b/mobile/android/android-components/components/support/webextensions/src/test/java/mozilla/components/support/webextensions/WebExtensionSupportTest.kt @@ -4,6 +4,7 @@ package mozilla.components.support.webextensions +import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.CustomTabListAction import mozilla.components.browser.state.action.EngineAction @@ -49,6 +50,7 @@ import org.junit.Assert.assertSame import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test +import org.junit.runner.RunWith import org.mockito.Mockito.never import org.mockito.Mockito.reset import org.mockito.Mockito.spy @@ -56,6 +58,7 @@ import org.mockito.Mockito.times import org.mockito.Mockito.verify import mozilla.components.support.base.facts.Action as FactsAction +@RunWith(AndroidJUnit4::class) class WebExtensionSupportTest { @get:Rule diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupController.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupController.kt @@ -187,10 +187,10 @@ class DefaultHistoryMetadataGroupController( override fun handleDeleteAllConfirmed() { scope.launch { - store.dispatch(HistoryMetadataGroupFragmentAction.DeleteAll) store.state.items.forEach { historyStorage.deleteVisitsFor(it.url) } + store.dispatch(HistoryMetadataGroupFragmentAction.DeleteAll) browserStore.dispatch( HistoryMetadataAction.DisbandSearchGroupAction(searchTerm = searchTerm), ) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt @@ -265,6 +265,7 @@ class BrowserFragmentTest { val newSelectedTab: TabSessionState = mockk(relaxed = true) every { newSelectedTab.content.loadRequest?.triggeredByRedirect } returns true + every { newSelectedTab.parentId } returns null browserFragment.observeTabSource(store) addAndSelectTab(newSelectedTab) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/fake/FakeMetricController.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/fake/FakeMetricController.kt @@ -14,6 +14,8 @@ import org.mozilla.fenix.components.metrics.MetricServiceType class FakeMetricController : MetricController { val startedServiceTypes: MutableList<MetricServiceType> = emptyList<MetricServiceType>().toMutableList() + val trackedEvents: MutableList<Event> = mutableListOf() + override fun start(type: MetricServiceType) { startedServiceTypes.add(type) } @@ -23,6 +25,6 @@ class FakeMetricController : MetricController { } override fun track(event: Event) { - // no-op + trackedEvents.add(event) } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddlewareTest.kt @@ -36,6 +36,7 @@ import mozilla.components.browser.state.action.ShareResourceAction import mozilla.components.browser.state.action.TabListAction.AddTabAction import mozilla.components.browser.state.action.TabListAction.RemoveTabAction import mozilla.components.browser.state.action.TrackingProtectionAction +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.ext.getUrl import mozilla.components.browser.state.search.RegionState import mozilla.components.browser.state.search.SearchEngine @@ -75,6 +76,8 @@ import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.store.ProgressBarConfig import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery +import mozilla.components.concept.engine.Engine +import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.EngineSession.LoadUrlFlags import mozilla.components.concept.engine.cookiehandling.CookieBannersStorage import mozilla.components.concept.engine.permission.SitePermissionsStorage @@ -1733,12 +1736,15 @@ class BrowserToolbarMiddlewareTest { every { settings.shouldUseBottomToolbar } returns false val currentTab = createTab("test.com", private = false) val captureMiddleware = CaptureActionsMiddleware<BrowserState, BrowserAction>() + val engine = mockk<Engine>(relaxed = true) + val session = mockk<EngineSession>(relaxed = true) + every { engine.createSession() } returns session val browserStore = BrowserStore( initialState = BrowserState( tabs = listOf(currentTab), selectedTabId = currentTab.id, ), - middleware = listOf(captureMiddleware), + middleware = listOf(captureMiddleware) + EngineMiddleware.create(engine), ) val middleware = buildMiddleware(appStore, browserStore = browserStore) val toolbarStore = buildStore(middleware) @@ -1774,7 +1780,7 @@ class BrowserToolbarMiddlewareTest { tabs = listOf(currentTab), selectedTabId = currentTab.id, ), - middleware = listOf(captureMiddleware), + middleware = listOf(captureMiddleware) + EngineMiddleware.create(mockk()), ) val middleware = buildMiddleware(appStore, browserStore = browserStore) val toolbarStore = buildStore(middleware) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt @@ -24,6 +24,7 @@ import kotlinx.coroutines.test.runTest import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.state.action.CustomTabListAction import mozilla.components.browser.state.action.ShareResourceAction +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ReaderState import mozilla.components.browser.state.state.TabSessionState @@ -156,11 +157,14 @@ class DefaultBrowserToolbarMenuControllerTest { every { browserAnimator.captureEngineViewAndDrawStatically(any(), capture(onComplete)) } answers { onComplete.captured.invoke(true) } selectedTab = createTab("https://www.mozilla.org", id = "1") + + val engineMiddleware = EngineMiddleware.create(mockk(), coroutinesTestRule.scope) browserStore = BrowserStore( initialState = BrowserState( tabs = listOf(selectedTab), selectedTabId = selectedTab.id, ), + engineMiddleware, ) } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/usecases/FenixBrowserUseCasesTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/usecases/FenixBrowserUseCasesTest.kt @@ -10,6 +10,7 @@ import io.mockk.mockk import io.mockk.spyk import io.mockk.verify import io.mockk.verifyOrder +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.search.SearchEngine import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.SearchState @@ -17,6 +18,7 @@ import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.base.profiler.Profiler +import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.utils.ABOUT_HOME_URL import mozilla.components.feature.search.SearchUseCases @@ -24,7 +26,9 @@ import mozilla.components.feature.search.ext.createSearchEngine import mozilla.components.feature.session.SessionUseCases import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R @@ -46,6 +50,9 @@ class FenixBrowserUseCasesTest { private lateinit var homepageTitle: String private lateinit var appStore: AppStore + @get:Rule + val coroutinesTestRule = MainCoroutineRule() + @Before fun setup() { addNewTabUseCase = mockk(relaxed = true) @@ -72,6 +79,10 @@ class FenixBrowserUseCasesTest { regionSearchEngines = listOf(searchEngine), ), ), + middleware = EngineMiddleware.create( + mockk<Engine>(), + coroutinesTestRule.scope, + ), ) defaultSearchUseCase = spyk( SearchUseCases( diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/downloads/listscreen/store/DownloadUIStoreTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/downloads/listscreen/store/DownloadUIStoreTest.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.downloads.listscreen.store import androidx.test.ext.junit.runners.AndroidJUnit4 import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.content.DownloadState import mozilla.components.browser.state.store.BrowserStore @@ -329,7 +330,7 @@ class DownloadUIStoreTest { assertEquals(store.state.pendingDeletionIds, deleteItemSet) assertEquals(expectedUIStateAfterDeleteAction, store.state) - dispatcher.scheduler.advanceTimeBy(UNDO_DELAY_PASSED.milliseconds) + dispatcher.scheduler.advanceTimeBy(testContext.getUndoDelay().milliseconds) assertEquals(store.state.pendingDeletionIds, deleteItemSet) assertEquals(expectedUIStateAfterDeleteAction, store.state) } @@ -364,6 +365,7 @@ class DownloadUIStoreTest { assertEquals(expectedUIStateBeforeDeleteAction, store.state) } + @OptIn(ExperimentalCoroutinesApi::class) @Test fun deleteOneElementAndCancelAfterDelayExpired() { val store = provideDownloadUIStore( @@ -375,22 +377,28 @@ class DownloadUIStoreTest { mode = DownloadUIState.Mode.Normal, pendingDeletionIds = emptySet(), ) - val expectedUIStateAfterDeleteAction = DownloadUIState( + val expectedUIStateAfterDeleteActionWithPendingDelete = DownloadUIState( items = listOf(fileItem1), mode = DownloadUIState.Mode.Normal, pendingDeletionIds = setOf("1"), ) + val expectedUIStateAfterDeleteActionAfterPendingDeleteTimeout = DownloadUIState( + items = listOf(fileItem1), + mode = DownloadUIState.Mode.Normal, + pendingDeletionIds = emptySet(), + ) + assertEquals(expectedUIStateBeforeDeleteAction, store.state) store.dispatch(DownloadUIAction.AddPendingDeletionSet(setOf("1"))) - assertEquals(expectedUIStateAfterDeleteAction, store.state) + assertEquals(expectedUIStateAfterDeleteActionWithPendingDelete, store.state) - dispatcher.scheduler.advanceTimeBy(UNDO_DELAY_PASSED.milliseconds) + dispatcher.scheduler.advanceTimeBy(testContext.getUndoDelay()) store.dispatch(DownloadUIAction.UndoPendingDeletion) dispatcher.scheduler.advanceUntilIdle() - assertEquals(expectedUIStateAfterDeleteAction, store.state) + assertEquals(expectedUIStateAfterDeleteActionAfterPendingDeleteTimeout, store.state) } @Test diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/PocketMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/PocketMiddlewareTest.kt @@ -9,8 +9,11 @@ import io.mockk.coVerify import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest import mozilla.components.service.pocket.PocketStoriesService import mozilla.components.service.pocket.PocketStory.ContentRecommendation import mozilla.components.service.pocket.PocketStory.PocketRecommendedStory @@ -146,10 +149,11 @@ class PocketMiddlewareTest { } } + @OptIn(ExperimentalCoroutinesApi::class) @Test - fun `WHEN PocketStoriesCategoriesChange is dispatched THEN intercept and dispatch PocketStoriesCategoriesSelectionsChange`() = runTestOnMain { + fun `WHEN PocketStoriesCategoriesChange is dispatched THEN intercept and dispatch PocketStoriesCategoriesSelectionsChange`() = runTest { val dataStore = FakeDataStore() - val currentCategories = listOf(mockk<PocketRecommendedStoriesCategory>()) + val currentCategories = listOf<PocketRecommendedStoriesCategory>() val pocketMiddleware = PocketMiddleware( mockk(), dataStore, @@ -170,6 +174,8 @@ class PocketMiddlewareTest { appStore.dispatch(ContentRecommendationsAction.PocketStoriesCategoriesChange(currentCategories)).joinBlocking() + advanceUntilIdle() + verify { appStore.dispatch( PocketStoriesCategoriesSelectionsChange( @@ -265,7 +271,7 @@ class PocketMiddlewareTest { @Test fun `WHEN restoreSelectedCategories is called THEN dispatch PocketStoriesCategoriesSelectionsChange with data read from the persistence layer`() = runTestOnMain { val dataStore = FakeDataStore("testCategory") - val currentCategories = listOf(mockk<PocketRecommendedStoriesCategory>()) + val currentCategories = listOf<PocketRecommendedStoriesCategory>() val captorMiddleware = CaptureActionsMiddleware<AppState, AppAction>() val appStore = AppStore( initialState = AppState(), diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/controller/DefaultRecentSyncedTabControllerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/controller/DefaultRecentSyncedTabControllerTest.kt @@ -12,6 +12,7 @@ import io.mockk.just import io.mockk.mockk import io.mockk.runs import io.mockk.verify +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ContentState import mozilla.components.browser.state.state.TabSessionState @@ -86,6 +87,7 @@ class DefaultRecentSyncedTabControllerTest { ), selectedTabId = nonSyncId, ), + middleware = EngineMiddleware.create(engine = mockk(relaxed = true)), ) val selectOrAddTabUseCase = TabsUseCases.SelectOrAddUseCase(store) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddlewareTest.kt @@ -700,7 +700,7 @@ class BrowserToolbarMiddlewareTest { browserStore.state.search.selectedOrDefaultSearchEngine, ) assertSearchSelectorEquals( - expectedSearchSelector(selectedSearchEngine), + expectedSearchSelector(selectedSearchEngine, listOf(otherSearchEngine)), toolbarStore.state.displayState.pageActionsStart[0] as SearchSelectorAction, ) } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/reviewprompt/ReviewPromptMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/reviewprompt/ReviewPromptMiddlewareTest.kt @@ -138,10 +138,12 @@ class ReviewPromptMiddlewareTest { @Test fun `GIVEN check ran WHEN check requested again THEN does nothing`() { - store.dispatch(ReviewPromptAction.CheckIfEligibleForReviewPrompt).joinBlocking() + mainCriteria = sequenceOf() + subCriteria = sequenceOf() + store.dispatch(ReviewPromptAction.CheckIfEligibleForReviewPrompt) val expectedState = store.state - store.dispatch(ReviewPromptAction.CheckIfEligibleForReviewPrompt).joinBlocking() + store.dispatch(ReviewPromptAction.CheckIfEligibleForReviewPrompt) assertEquals(expectedState, store.state) } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/trustpanel/TrustPanelMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/trustpanel/TrustPanelMiddlewareTest.kt @@ -315,6 +315,13 @@ class TrustPanelMiddlewareTest { trustPanelState = TrustPanelState( sitePermissions = null, sessionState = sessionState, + websitePermissionsState = mapOf( + PhoneFeature.AUTOPLAY to WebsitePermission.Autoplay( + autoplayValue = AutoplayValue.AUTOPLAY_BLOCK_AUDIBLE, + isVisible = true, + deviceFeature = PhoneFeature.CAMERA, + ), + ), ), ) @@ -352,6 +359,13 @@ class TrustPanelMiddlewareTest { trustPanelState = TrustPanelState( sitePermissions = originalSitePermissions, sessionState = sessionState, + websitePermissionsState = mapOf( + PhoneFeature.AUTOPLAY to WebsitePermission.Autoplay( + autoplayValue = AutoplayValue.AUTOPLAY_BLOCK_AUDIBLE, + isVisible = true, + deviceFeature = PhoneFeature.CAMERA, + ), + ), ), ) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/share/SaveToPDFMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/share/SaveToPDFMiddlewareTest.kt @@ -10,9 +10,11 @@ import io.mockk.just import io.mockk.mockk import io.mockk.verify import mozilla.components.browser.state.action.EngineAction +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.store.BrowserStore +import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.EngineSession import mozilla.components.support.test.libstate.ext.waitUntilIdle import mozilla.components.support.test.robolectric.testContext @@ -231,8 +233,12 @@ class SaveToPDFMiddlewareTest { firstArg<(Boolean) -> Unit>().invoke(false) } } + val engineMiddleware = EngineMiddleware.create( + mockk<Engine>(), + mainCoroutineTestRule.scope, + ) val browserStore = BrowserStore( - middleware = listOf(middleware), + middleware = listOf(middleware) + engineMiddleware, initialState = BrowserState( tabs = listOf( createTab( @@ -394,8 +400,12 @@ class SaveToPDFMiddlewareTest { firstArg<(Boolean) -> Unit>().invoke(false) } } + val engineMiddleware = EngineMiddleware.create( + mockk<Engine>(), + mainCoroutineTestRule.scope, + ) val browserStore = BrowserStore( - middleware = listOf(middleware), + middleware = listOf(middleware) + engineMiddleware, initialState = BrowserState( tabs = listOf( createTab( diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabhistory/TabHistoryControllerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabhistory/TabHistoryControllerTest.kt @@ -8,9 +8,13 @@ import androidx.navigation.NavController import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.concept.engine.Engine import mozilla.components.feature.session.SessionUseCases +import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Before +import org.junit.Rule import org.junit.Test class TabHistoryControllerTest { @@ -20,9 +24,16 @@ class TabHistoryControllerTest { private lateinit var currentItem: TabHistoryItem private lateinit var store: BrowserStore + @get:Rule + val coroutinesTestRule = MainCoroutineRule() + @Before fun setUp() { - store = BrowserStore() + val engineMiddleware = EngineMiddleware.create( + mockk<Engine>(), + coroutinesTestRule.scope, + ) + store = BrowserStore(middleware = engineMiddleware) navController = mockk(relaxed = true) goToHistoryIndexUseCase = spyk(SessionUseCases(store).goToHistoryIndex) currentItem = TabHistoryItem( 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 @@ -16,6 +16,7 @@ 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 @@ -44,6 +45,7 @@ import org.mozilla.fenix.GleanMetrics.Metrics import org.mozilla.fenix.GleanMetrics.Translations import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.components.fake.FakeMetricController import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components @@ -66,7 +68,7 @@ class TelemetryMiddlewareTest { val gleanRule = FenixGleanTestRule(ApplicationProvider.getApplicationContext()) private val clock = FakeClock() - private val metrics: MetricController = mockk() + private val metrics = FakeMetricController() @Before fun setUp() { @@ -85,7 +87,7 @@ class TelemetryMiddlewareTest { every { engine.createSession(any(), any()) } returns mockk(relaxed = true) store = BrowserStore( - middleware = listOf(telemetryMiddleware), + middleware = listOf(telemetryMiddleware) + EngineMiddleware.create(engine), initialState = BrowserState(), ) appStore = AppStore() @@ -447,7 +449,7 @@ class TelemetryMiddlewareTest { 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) } + assertTrue(metrics.trackedEvents.contains(Event.GrowthData.FirstUriLoadForDay)) } @Test diff --git a/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/cfr/CfrMiddlewareTest.kt b/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/cfr/CfrMiddlewareTest.kt @@ -94,7 +94,7 @@ class CfrMiddlewareTest { @Test fun `GIVEN mozilla tab WHEN UpdateSecurityInfoAction is intercepted THEN showTrackingProtectionCfr is not changed to true`() { if (onboardingExperiment.isCfrEnabled) { - val mozillaTab = createTab(id = "1", url = "https://www.mozilla.org") + val mozillaTab = createTab(tabId = 1, tabUrl = "https://www.mozilla.org") val updateSecurityInfoAction = ContentAction.UpdateSecurityInfoAction( "1", SecurityInfoState( diff --git a/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/searchwidget/ExternalIntentNavigationTest.kt b/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/searchwidget/ExternalIntentNavigationTest.kt @@ -100,7 +100,7 @@ internal class ExternalIntentNavigationTest { @Test fun `GIVEN a tab is already open WHEN trying to navigate to the current tab THEN navigate to it and return true`() { - activity.components.tabsUseCases.addTab(url = "https://mozilla.com") + activity.components.tabsUseCases.addTab(url = "https://mozilla.com", private = true) activity.components.store.waitUntilIdle() val result = ExternalIntentNavigation.handleBrowserTabAlreadyOpen(activity) activity.components.appStore.waitUntilIdle()