[fenix] Fix intermittent test failures in BookmarkControllerTest

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt

@@ -4,7 +4,6 @@
 
 package org.mozilla.fenix.library.bookmarks
 
-import android.content.ClipData
 import android.content.ClipboardManager
 import android.content.Context
 import androidx.navigation.NavController
@@ -30,6 +29,7 @@ import mozilla.components.concept.storage.BookmarkNodeType
 import mozilla.components.feature.tabs.TabsUseCases
 import org.junit.After
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.mozilla.fenix.BrowserDirection
@@ -45,8 +45,6 @@ import org.mozilla.fenix.ext.components
 @ExperimentalCoroutinesApi
 class BookmarkControllerTest {
 
-    private lateinit var controller: BookmarkController
-
     private val bookmarkStore = spyk(BookmarkFragmentStore(BookmarkFragmentState(null)))
     private val context: Context = mockk(relaxed = true)
     private val scope = TestCoroutineScope()
@@ -54,13 +52,6 @@ class BookmarkControllerTest {
     private val navController: NavController = mockk(relaxed = true)
     private val sharedViewModel: BookmarksSharedViewModel = mockk()
     private val tabsUseCases: TabsUseCases = mockk()
-    private val loadBookmarkNode: suspend (String) -> BookmarkNode? = mockk(relaxed = true)
-    private val showSnackbar: (String) -> Unit = mockk(relaxed = true)
-    private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true)
-    private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = mockk(relaxed = true)
-    private val invokePendingDeletion: () -> Unit = mockk(relaxed = true)
-    private val showTabTray: () -> Unit = mockk(relaxed = true)
-
     private val homeActivity: HomeActivity = mockk(relaxed = true)
     private val services: Services = mockk(relaxed = true)
     private val addNewTabUseCase: TabsUseCases.AddNewTabUseCase = mockk(relaxed = true)
@@ -102,22 +93,6 @@ class BookmarkControllerTest {
         every { bookmarkStore.dispatch(any()) } returns mockk()
         every { sharedViewModel.selectedFolder = any() } just runs
         every { tabsUseCases.addTab } returns addNewTabUseCase
-
-        controller = DefaultBookmarkController(
-            activity = homeActivity,
-            navController = navController,
-            clipboardManager = clipboardManager,
-            scope = scope,
-            store = bookmarkStore,
-            sharedViewModel = sharedViewModel,
-            tabsUseCases = tabsUseCases,
-            loadBookmarkNode = loadBookmarkNode,
-            showSnackbar = showSnackbar,
-            deleteBookmarkNodes = deleteBookmarkNodes,
-            deleteBookmarkFolder = deleteBookmarkFolder,
-            invokePendingDeletion = invokePendingDeletion,
-            showTabTray = showTabTray
-        )
     }
 
     @After
@@ -127,7 +102,7 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleBookmarkChanged updates the selected bookmark node`() {
-        controller.handleBookmarkChanged(tree)
+        createController().handleBookmarkChanged(tree)
 
         verify {
             sharedViewModel.selectedFolder = tree
@@ -137,10 +112,13 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleBookmarkTapped should load the bookmark in a new tab`() {
-        controller.handleBookmarkTapped(item)
+        var invokePendingDeletionInvoked = false
+        createController(invokePendingDeletion = {
+            invokePendingDeletionInvoked = true
+        }).handleBookmarkTapped(item)
 
-        verifyOrder {
+        assertTrue(invokePendingDeletionInvoked)
-            invokePendingDeletion.invoke()
+        verify {
             homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks)
         }
     }
@@ -150,6 +128,7 @@ class BookmarkControllerTest {
         // if in normal mode, should be in normal mode
         every { homeActivity.browsingModeManager.mode } returns BrowsingMode.Normal
 
+        val controller = createController()
         controller.handleBookmarkTapped(item)
         assertEquals(BrowsingMode.Normal, homeActivity.browsingModeManager.mode)
 
@@ -162,24 +141,24 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleBookmarkExpand clears selection and invokes pending deletions`() {
-        coEvery { loadBookmarkNode.invoke(any()) } returns tree
+        var invokePendingDeletionInvoked = false
-
+        createController(invokePendingDeletion = {
-        controller.handleBookmarkExpand(tree)
+            invokePendingDeletionInvoked = true
+        }).handleBookmarkExpand(tree)
 
-        verify {
+        assertTrue(invokePendingDeletionInvoked)
-            invokePendingDeletion.invoke()
-            controller.handleAllBookmarksDeselected()
-        }
     }
 
     @Test
     fun `handleBookmarkExpand should refresh and change the active bookmark node`() {
-        coEvery { loadBookmarkNode.invoke(any()) } returns tree
+        var loadBookmarkNodeInvoked = false
-
+        createController(loadBookmarkNode = {
-        controller.handleBookmarkExpand(tree)
+            loadBookmarkNodeInvoked = true
+            tree
+        }).handleBookmarkExpand(tree)
 
+        assertTrue(loadBookmarkNodeInvoked)
         coVerify {
-            loadBookmarkNode.invoke(tree.guid)
             sharedViewModel.selectedFolder = tree
             bookmarkStore.dispatch(BookmarkFragmentAction.Change(tree))
         }
@@ -187,7 +166,7 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleSelectionModeSwitch should invalidateOptionsMenu`() {
-        controller.handleSelectionModeSwitch()
+        createController().handleSelectionModeSwitch()
 
         verify {
             homeActivity.invalidateOptionsMenu()
@@ -196,10 +175,13 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleBookmarkEdit should navigate to the 'Edit' fragment`() {
-        controller.handleBookmarkEdit(item)
+        var invokePendingDeletionInvoked = false
+        createController(invokePendingDeletion = {
+            invokePendingDeletionInvoked = true
+        }).handleBookmarkEdit(item)
 
+        assertTrue(invokePendingDeletionInvoked)
         verify {
-            invokePendingDeletion.invoke()
             navController.navigate(
                 BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(
                     item.guid
@@ -211,7 +193,7 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleBookmarkSelected dispatches Select action when selecting a non-root folder`() {
-        controller.handleBookmarkSelected(item)
+        createController().handleBookmarkSelected(item)
 
         verify {
             bookmarkStore.dispatch(BookmarkFragmentAction.Select(item))
@@ -222,25 +204,27 @@ class BookmarkControllerTest {
     fun `handleBookmarkSelected should show a toast when selecting a root folder`() {
         val errorMessage = context.getString(R.string.bookmark_cannot_edit_root)
 
-        controller.handleBookmarkSelected(root)
+        var showSnackbarInvoked = false
+        createController(showSnackbar = {
+            assertEquals(errorMessage, it)
+            showSnackbarInvoked = true
+        }).handleBookmarkSelected(root)
 
-        verify {
+        assertTrue(showSnackbarInvoked)
-            showSnackbar(errorMessage)
-        }
     }
 
     @Test
     fun `handleBookmarkSelected does not select in Syncing mode`() {
         every { bookmarkStore.state.mode } returns BookmarkFragmentState.Mode.Syncing
 
-        controller.handleBookmarkSelected(item)
+        createController().handleBookmarkSelected(item)
 
         verify { bookmarkStore.dispatch(BookmarkFragmentAction.Select(item)) wasNot called }
     }
 
     @Test
     fun `handleBookmarkDeselected dispatches Deselect action`() {
-        controller.handleBookmarkDeselected(item)
+        createController().handleBookmarkDeselected(item)
 
         verify {
             bookmarkStore.dispatch(BookmarkFragmentAction.Deselect(item))
@@ -251,12 +235,13 @@ class BookmarkControllerTest {
     fun `handleCopyUrl should copy bookmark url to clipboard and show a toast`() {
         val urlCopiedMessage = context.getString(R.string.url_copied)
 
-        controller.handleCopyUrl(item)
+        var showSnackbarInvoked = false
+        createController(showSnackbar = {
+            assertEquals(urlCopiedMessage, it)
+            showSnackbarInvoked = true
+        }).handleCopyUrl(item)
 
-        verifyOrder {
+        assertTrue(showSnackbarInvoked)
-            ClipData.newPlainText(item.url, item.url)
-            showSnackbar(urlCopiedMessage)
-        }
     }
 
     @Test
@@ -264,7 +249,7 @@ class BookmarkControllerTest {
         val navDirectionsSlot = slot<NavDirections>()
         every { navController.navigate(capture(navDirectionsSlot), null) } just Runs
 
-        controller.handleBookmarkSharing(item)
+        createController().handleBookmarkSharing(item)
 
         verify {
             navController.navigate(navDirectionsSlot.captured, null)
@@ -273,72 +258,96 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleBookmarkTapped should open the bookmark`() {
-        controller.handleBookmarkTapped(item)
+        var invokePendingDeletionInvoked = false
+        createController(invokePendingDeletion = {
+            invokePendingDeletionInvoked = true
+        }).handleBookmarkTapped(item)
 
-        verifyOrder {
+        assertTrue(invokePendingDeletionInvoked)
-            invokePendingDeletion.invoke()
+        verify {
             homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks)
         }
     }
 
     @Test
     fun `handleOpeningBookmark should open the bookmark a new 'Normal' tab`() {
-        controller.handleOpeningBookmark(item, BrowsingMode.Normal)
+        var invokePendingDeletionInvoked = false
+        var showTabTrayInvoked = false
+        createController(invokePendingDeletion = {
+            invokePendingDeletionInvoked = true
+        }, showTabTray = {
+            showTabTrayInvoked = true
+        }
+        ).handleOpeningBookmark(item, BrowsingMode.Normal)
 
+        assertTrue(invokePendingDeletionInvoked)
+        assertTrue(showTabTrayInvoked)
         verifyOrder {
-            invokePendingDeletion.invoke()
             homeActivity.browsingModeManager.mode = BrowsingMode.Normal
             addNewTabUseCase.invoke(item.url!!, private = false)
-            showTabTray
         }
     }
 
     @Test
     fun `handleOpeningBookmark should open the bookmark a new 'Private' tab`() {
-        controller.handleOpeningBookmark(item, BrowsingMode.Private)
+        var invokePendingDeletionInvoked = false
+        var showTabTrayInvoked = false
+        createController(invokePendingDeletion = {
+            invokePendingDeletionInvoked = true
+        }, showTabTray = {
+            showTabTrayInvoked = true
+        }
+        ).handleOpeningBookmark(item, BrowsingMode.Private)
 
+        assertTrue(invokePendingDeletionInvoked)
+        assertTrue(showTabTrayInvoked)
         verifyOrder {
-            invokePendingDeletion.invoke()
             homeActivity.browsingModeManager.mode = BrowsingMode.Private
             addNewTabUseCase.invoke(item.url!!, private = true)
-            showTabTray
         }
     }
 
     @Test
     fun `handleBookmarkDeletion for an item should properly call a passed in delegate`() {
-        controller.handleBookmarkDeletion(setOf(item), Event.RemoveBookmark)
+        var deleteBookmarkNodesInvoked = false
-
+        createController(deleteBookmarkNodes = { nodes, event ->
-        verify {
+            assertEquals(setOf(item), nodes)
-            deleteBookmarkNodes(setOf(item), Event.RemoveBookmark)
+            assertEquals(Event.RemoveBookmark, event)
-        }
+            deleteBookmarkNodesInvoked = true
+        }).handleBookmarkDeletion(setOf(item), Event.RemoveBookmark)
+
+        assertTrue(deleteBookmarkNodesInvoked)
     }
 
     @Test
     fun `handleBookmarkDeletion for multiple bookmarks should properly call a passed in delegate`() {
-        controller.handleBookmarkDeletion(setOf(item, subfolder), Event.RemoveBookmarks)
+        var deleteBookmarkNodesInvoked = false
-
+        createController(deleteBookmarkNodes = { nodes, event ->
-        verify {
+            assertEquals(setOf(item, subfolder), nodes)
-            deleteBookmarkNodes(setOf(item, subfolder), Event.RemoveBookmarks)
+            assertEquals(Event.RemoveBookmarks, event)
-        }
+            deleteBookmarkNodesInvoked = true
+        }).handleBookmarkDeletion(setOf(item, subfolder), Event.RemoveBookmarks)
+
+        assertTrue(deleteBookmarkNodesInvoked)
     }
 
     @Test
     fun `handleBookmarkDeletion for a folder should properly call the delete folder delegate`() {
-        controller.handleBookmarkFolderDeletion(setOf(subfolder))
+        var deleteBookmarkFolderInvoked = false
+        createController(deleteBookmarkFolder = { nodes ->
+            assertEquals(setOf(subfolder), nodes)
+            deleteBookmarkFolderInvoked = true
+        }).handleBookmarkFolderDeletion(setOf(subfolder))
 
-        verify {
+        assertTrue(deleteBookmarkFolderInvoked)
-            deleteBookmarkFolder(setOf(subfolder))
-        }
     }
 
     @Test
     fun `handleRequestSync dispatches actions in the correct order`() {
         every { homeActivity.components.backgroundServices.accountManager } returns mockk(relaxed = true)
         coEvery { homeActivity.bookmarkStorage.getBookmark(any()) } returns tree
-        coEvery { loadBookmarkNode.invoke(any()) } returns tree
 
-        controller.handleRequestSync()
+        createController().handleRequestSync()
 
         verifyOrder {
             bookmarkStore.dispatch(BookmarkFragmentAction.StartSync)
@@ -351,11 +360,41 @@ class BookmarkControllerTest {
         every { bookmarkStore.state.guidBackstack } returns listOf(tree.guid)
         every { bookmarkStore.state.tree } returns tree
 
-        controller.handleBackPressed()
+        var invokePendingDeletionInvoked = false
+        createController(invokePendingDeletion = {
+            invokePendingDeletionInvoked = true
+        }).handleBackPressed()
+
+        assertTrue(invokePendingDeletionInvoked)
 
         verify {
-            invokePendingDeletion.invoke()
             navController.popBackStack()
         }
     }
+
+    @Suppress("LongParameterList")
+    private fun createController(
+        loadBookmarkNode: suspend (String) -> BookmarkNode? = { _ -> null },
+        showSnackbar: (String) -> Unit = { _ -> },
+        deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = { _, _ -> },
+        deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = { _ -> },
+        invokePendingDeletion: () -> Unit = { },
+        showTabTray: () -> Unit = { }
+    ): BookmarkController {
+        return DefaultBookmarkController(
+            activity = homeActivity,
+            navController = navController,
+            clipboardManager = clipboardManager,
+            scope = scope,
+            store = bookmarkStore,
+            sharedViewModel = sharedViewModel,
+            tabsUseCases = tabsUseCases,
+            loadBookmarkNode = loadBookmarkNode,
+            showSnackbar = showSnackbar,
+            deleteBookmarkNodes = deleteBookmarkNodes,
+            deleteBookmarkFolder = deleteBookmarkFolder,
+            invokePendingDeletion = invokePendingDeletion,
+            showTabTray = showTabTray
+        )
+    }
 }