Closes mozilla-mobile#25954: fix failing HistoryMetadataControllerTes…

…t.kt tests, remove skipping scoped tests

app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt

@@ -23,6 +23,7 @@ import org.mozilla.fenix.components.appstate.AppAction
 import org.mozilla.fenix.components.history.DefaultPagedHistoryProvider
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.navigateSafe
+import org.mozilla.fenix.library.history.HistoryFragment.DeleteConfirmationDialogFragment
 import org.mozilla.fenix.utils.Settings
 import org.mozilla.fenix.GleanMetrics.History as GleanHistory
 
@@ -34,19 +35,22 @@ interface HistoryController {
     fun handleBackPressed(): Boolean
     fun handleModeSwitched()
     fun handleSearch()
+
     /**
-     * Displays a [HistoryFragment.DeleteConfirmationDialogFragment].
+     * Displays a [DeleteConfirmationDialogFragment].
      */
     fun handleDeleteTimeRange()
     fun handleDeleteSome(items: Set<History>)
+
     /**
-     * Removes history items inside the time frame.
+     * Deletes history items inside the time frame.
      *
      * @param timeFrame Selected time frame by the user. If `null`, removes all history.
      */
     fun handleDeleteTimeRangeConfirmed(timeFrame: RemoveTimeFrame?)
     fun handleRequestSync()
     fun handleEnterRecentlyClosed()
+
     /**
      * Navigates to [org.mozilla.fenix.library.syncedhistory.SyncedHistoryFragment]
      */

app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt

@@ -98,6 +98,7 @@ class HistoryMetadataGroupFragment :
                 store = historyMetadataGroupStore,
                 selectOrAddUseCase = requireComponents.useCases.tabsUseCases.selectOrAddTab,
                 navController = findNavController(),
+                scope = CoroutineScope(Dispatchers.IO),
                 searchTerm = args.title,
                 deleteSnackbar = :: deleteSnackbar,
                 promptDeleteAll = :: promptDeleteAll,
@@ -195,7 +196,7 @@ class HistoryMetadataGroupFragment :
                 true
             }
             R.id.history_delete -> {
-                interactor.onDeleteAllMenuItem()
+                interactor.onDeleteAll()
                 true
             }
             else -> super.onOptionsItemSelected(item)
@@ -218,14 +219,14 @@ class HistoryMetadataGroupFragment :
         )
     }
 
-    private fun promptDeleteAll(delete: () -> Unit) {
+    private fun promptDeleteAll() {
         if (childFragmentManager.findFragmentByTag(DeleteAllConfirmationDialogFragment.TAG)
             as? DeleteAllConfirmationDialogFragment != null
         ) {
             return
         }
 
-        DeleteAllConfirmationDialogFragment(delete, args.title).show(
+        DeleteAllConfirmationDialogFragment(interactor, args.title).show(
             childFragmentManager, DeleteAllConfirmationDialogFragment.TAG
         )
     }
@@ -255,7 +256,7 @@ class HistoryMetadataGroupFragment :
     }
 
     internal class DeleteAllConfirmationDialogFragment(
-        private val delete: () -> Unit,
+        private val interactor: HistoryMetadataGroupInteractor,
         private val groupName: String
     ) : DialogFragment() {
         override fun onCreateDialog(savedInstanceState: Bundle?): Dialog =
@@ -270,7 +271,7 @@ class HistoryMetadataGroupFragment :
                     dialog.cancel()
                 }
                 .setPositiveButton(R.string.delete_history_group_prompt_allow) { dialog: DialogInterface, _ ->
-                    delete.invoke()
+                    interactor.onDeleteAllConfirmed()
                     dialog.dismiss()
                 }
                 .create()

app/src/main/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupController.kt

@@ -7,7 +7,6 @@ package org.mozilla.fenix.library.historymetadata.controller
 import android.content.Context
 import androidx.navigation.NavController
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers.IO
 import kotlinx.coroutines.Dispatchers.Main
 import kotlinx.coroutines.launch
 import mozilla.components.browser.state.action.HistoryMetadataAction
@@ -22,6 +21,7 @@ import org.mozilla.fenix.ext.components
 import mozilla.components.service.glean.private.NoExtras
 import org.mozilla.fenix.library.history.History
 import org.mozilla.fenix.library.history.toPendingDeletionHistory
+import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragment.DeleteAllConfirmationDialogFragment
 import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentAction
 import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentDirections
 import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentStore
@@ -74,9 +74,14 @@ interface HistoryMetadataGroupController {
     fun handleDelete(items: Set<History.Metadata>)
 
     /**
-     * Deletes all the history metadata items in this group.
+     * Displays a [DeleteAllConfirmationDialogFragment] prompt.
      */
     fun handleDeleteAll()
+
+    /**
+     * Deletes history metadata items in this group.
+     */
+    fun handleDeleteAllConfirmed()
 }
 
 /**
@@ -90,13 +95,14 @@ class DefaultHistoryMetadataGroupController(
     private val store: HistoryMetadataGroupFragmentStore,
     private val selectOrAddUseCase: TabsUseCases.SelectOrAddUseCase,
     private val navController: NavController,
+    private val scope: CoroutineScope,
     private val searchTerm: String,
     private val deleteSnackbar: (
         items: Set<History.Metadata>,
         undo: suspend (Set<History.Metadata>) -> Unit,
         delete: (Set<History.Metadata>) -> suspend (context: Context) -> Unit
     ) -> Unit,
-    private val promptDeleteAll: (() -> Unit) -> Unit,
+    private val promptDeleteAll: () -> Unit,
     private val allDeletedSnackbar: () -> Unit
 ) : HistoryMetadataGroupController {
 
@@ -144,7 +150,7 @@ class DefaultHistoryMetadataGroupController(
 
     private fun delete(items: Set<History.Metadata>): suspend (context: Context) -> Unit {
         return { context ->
-            CoroutineScope(IO).launch {
+            scope.launch {
                 val isDeletingLastItem = items.containsAll(store.state.items)
                 items.forEach {
                     store.dispatch(HistoryMetadataGroupFragmentAction.Delete(it))
@@ -163,11 +169,11 @@ class DefaultHistoryMetadataGroupController(
     }
 
     override fun handleDeleteAll() {
-        promptDeleteAll.invoke(::deleteAll)
+        promptDeleteAll.invoke()
     }
 
-    private fun deleteAll() {
-        CoroutineScope(IO).launch {
+    override fun handleDeleteAllConfirmed() {
+        scope.launch {
             store.dispatch(HistoryMetadataGroupFragmentAction.DeleteAll)
             store.state.items.forEach {
                 historyStorage.deleteVisitsFor(it.url)

app/src/main/java/org/mozilla/fenix/library/historymetadata/interactor/HistoryMetadataGroupInteractor.kt

@@ -29,10 +29,14 @@ interface HistoryMetadataGroupInteractor : SelectionInteractor<History.Metadata>
     fun onDelete(items: Set<History.Metadata>)
 
     /**
-     * Deletes all the history items in the history metadata group. Called when a user clicks
-     * on the "Delete history" menu item.
+     * Called when a user clicks on the "Delete history" menu item.
      */
-    fun onDeleteAllMenuItem()
+    fun onDeleteAll()
+
+    /**
+     * Called when a user has confirmed the deletion of the group.
+     */
+    fun onDeleteAllConfirmed()
 
     /**
      * Opens the share sheet for a set of history [items]. Called when a user clicks on the
@@ -70,10 +74,14 @@ class DefaultHistoryMetadataGroupInteractor(
         controller.handleDelete(items)
     }
 
-    override fun onDeleteAllMenuItem() {
+    override fun onDeleteAll() {
         controller.handleDeleteAll()
     }
 
+    override fun onDeleteAllConfirmed() {
+        controller.handleDeleteAllConfirmed()
+    }
+
     override fun onShareMenuItem(items: Set<History.Metadata>) {
         controller.handleShare(items)
     }

app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt

@@ -23,11 +23,10 @@ import mozilla.components.support.test.rule.MainCoroutineRule
 import mozilla.components.support.test.rule.runTestOnMain
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
-import org.junit.Assert.assertNull
 import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
-import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -90,23 +89,7 @@ class HistoryMetadataGroupControllerTest {
 
     @Before
     fun setUp() {
-        controller = DefaultHistoryMetadataGroupController(
-            historyStorage = historyStorage,
-            browserStore = browserStore,
-            appStore = appStore,
-            store = store,
-            selectOrAddUseCase = selectOrAddUseCase,
-            navController = navController,
-            searchTerm = "mozilla",
-            deleteSnackbar = { items, _, delete ->
-                scope.launch {
-                    delete(items).invoke(context)
-                }
-            },
-            promptDeleteAll = { deleteAll -> deleteAll.invoke() },
-            allDeletedSnackbar = {}
-        )
-
+        controller = createController()
         every { activity.components.core.historyStorage } returns historyStorage
         every { context.components.core.store } returns browserStore
         every { context.components.core.historyStorage } returns historyStorage
@@ -189,7 +172,6 @@ class HistoryMetadataGroupControllerTest {
     }
 
     @Test
-    @Ignore("Intermittent test: https://github.com/mozilla-mobile/fenix/issues/25167")
     fun handleDeleteSingle() = runTestOnMain {
         assertNull(GleanHistory.searchTermGroupRemoveTab.testGetValue())
 
@@ -218,7 +200,6 @@ class HistoryMetadataGroupControllerTest {
     }
 
     @Test
-    @Ignore("Intermittent test: https://github.com/mozilla-mobile/fenix/issues/25167")
     fun handleDeleteMultiple() = runTestOnMain {
         assertNull(GleanHistory.searchTermGroupRemoveTab.testGetValue())
         controller.handleDelete(getMetadataItemsList().toSet())
@@ -244,14 +225,16 @@ class HistoryMetadataGroupControllerTest {
     }
 
     @Test
-    @Ignore("Intermittent test: https://github.com/mozilla-mobile/fenix/issues/25167")
     fun handleDeleteAbnormal() = runTestOnMain {
         val abnormalList = listOf(
             mozillaHistoryMetadataItem,
             firefoxHistoryMetadataItem,
             mozillaHistoryMetadataItem.copy(title = "Pocket", url = "https://getpocket.com"),
             mozillaHistoryMetadataItem.copy(title = "BBC", url = "https://www.bbc.com/"),
-            mozillaHistoryMetadataItem.copy(title = "Stackoverflow", url = "https://stackoverflow.com/")
+            mozillaHistoryMetadataItem.copy(
+                title = "Stackoverflow",
+                url = "https://stackoverflow.com/"
+            )
         )
         assertNull(GleanHistory.searchTermGroupRemoveTab.testGetValue())
 
@@ -290,9 +273,21 @@ class HistoryMetadataGroupControllerTest {
 
     @Test
     fun handleDeleteAll() = runTestOnMain {
+        var promptDeleteAllInvoked = false
+        val controller = createController(
+            promptDeleteAll = {
+                promptDeleteAllInvoked = true
+            }
+        )
+        controller.handleDeleteAll()
+        assertTrue(promptDeleteAllInvoked)
+    }
+
+    @Test
+    fun handleDeleteAllConfirmed() = runTestOnMain {
         assertNull(GleanHistory.searchTermGroupRemoveAll.testGetValue())
 
-        controller.handleDeleteAll()
+        controller.handleDeleteAllConfirmed()
 
         coVerify {
             store.dispatch(HistoryMetadataGroupFragmentAction.DeleteAll)
@@ -313,4 +308,33 @@ class HistoryMetadataGroupControllerTest {
                 .single().extra
         )
     }
+
+    @Suppress("LongParameterList")
+    private fun createController(
+        deleteSnackbar: (
+            items: Set<History.Metadata>,
+            undo: suspend (Set<History.Metadata>) -> Unit,
+            delete: (Set<History.Metadata>) -> suspend (context: Context) -> Unit
+        ) -> Unit = { items, _, delete ->
+            scope.launch {
+                delete(items).invoke(context)
+            }
+        },
+        promptDeleteAll: () -> Unit = {},
+        allDeletedSnackbar: () -> Unit = {}
+    ): DefaultHistoryMetadataGroupController {
+        return DefaultHistoryMetadataGroupController(
+            historyStorage = historyStorage,
+            browserStore = browserStore,
+            appStore = appStore,
+            store = store,
+            selectOrAddUseCase = selectOrAddUseCase,
+            navController = navController,
+            scope = scope,
+            searchTerm = searchTerm,
+            deleteSnackbar = deleteSnackbar,
+            promptDeleteAll = promptDeleteAll,
+            allDeletedSnackbar = allDeletedSnackbar
+        )
+    }
 }