Add tests for collection creation controller

app/src/main/java/org/mozilla/fenix/collections/CollectionCreationController.kt

@@ -8,14 +8,14 @@ package org.mozilla.fenix.collections
 
 import androidx.annotation.VisibleForTesting
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Dispatchers.IO
 import kotlinx.coroutines.launch
 import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
 import mozilla.components.feature.tab.collections.TabCollection
-import org.mozilla.fenix.components.Analytics
 import org.mozilla.fenix.components.TabCollectionStorage
 import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.home.Tab
 
 interface CollectionCreationController {
@@ -65,10 +65,10 @@ fun List<Tab>.toSessionBundle(sessionManager: SessionManager): List<Session> {
 class DefaultCollectionCreationController(
     private val store: CollectionCreationStore,
     private val dismiss: () -> Unit,
-    private val analytics: Analytics,
+    private val metrics: MetricController,
     private val tabCollectionStorage: TabCollectionStorage,
     private val sessionManager: SessionManager,
-    private val viewLifecycleScope: CoroutineScope
+    private val scope: CoroutineScope
 ) : CollectionCreationController {
 
     companion object {
@@ -79,26 +79,31 @@ class DefaultCollectionCreationController(
     override fun saveCollectionName(tabs: List<Tab>, name: String) {
         dismiss()
 
-        val sessionBundle = tabs.toList().toSessionBundle(sessionManager)
+        val sessionBundle = tabs.toSessionBundle(sessionManager)
-        viewLifecycleScope.launch(Dispatchers.IO) {
+        scope.launch(IO) {
             tabCollectionStorage.createCollection(name, sessionBundle)
         }
 
-        analytics.metrics.track(
+        metrics.track(
             Event.CollectionSaved(normalSessionSize(sessionManager), sessionBundle.size)
         )
     }
 
     override fun renameCollection(collection: TabCollection, name: String) {
         dismiss()
-        viewLifecycleScope.launch(Dispatchers.IO) {
+        scope.launch(IO) {
             tabCollectionStorage.renameCollection(collection, name)
-            analytics.metrics.track(Event.CollectionRenamed)
         }
+        metrics.track(Event.CollectionRenamed)
     }
 
     override fun backPressed(fromStep: SaveCollectionStep) {
-        handleBackPress(fromStep)
+        val newStep = stepBack(fromStep)
+        if (newStep != null) {
+            store.dispatch(CollectionCreationAction.StepChanged(newStep))
+        } else {
+            dismiss()
+        }
     }
 
     override fun selectAllTabs() {
@@ -116,12 +121,12 @@ class DefaultCollectionCreationController(
     override fun selectCollection(collection: TabCollection, tabs: List<Tab>) {
         dismiss()
         val sessionBundle = tabs.toList().toSessionBundle(sessionManager)
-        viewLifecycleScope.launch(Dispatchers.IO) {
+        scope.launch(IO) {
             tabCollectionStorage
                 .addTabsToCollection(collection, sessionBundle)
         }
 
-        analytics.metrics.track(
+        metrics.track(
             Event.CollectionTabsAdded(normalSessionSize(sessionManager), sessionBundle.size)
         )
     }
@@ -171,25 +176,15 @@ class DefaultCollectionCreationController(
         store.dispatch(CollectionCreationAction.TabRemoved(tab))
     }
 
-    private fun handleBackPress(backFromStep: SaveCollectionStep) {
+    /**
-        val newStep = stepBack(backFromStep)
+     * Will return the next valid state according to this diagram.
-        if (newStep != null) {
+     *
-            store.dispatch(CollectionCreationAction.StepChanged(newStep))
+     * Name Collection -> Select Collection -> Select Tabs -> (dismiss fragment) <- Rename Collection
-        } else {
+     */
-            dismiss()
-        }
-    }
-
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     fun stepBack(
         backFromStep: SaveCollectionStep
     ): SaveCollectionStep? {
-        /*
-        Will return the next valid state according to this diagram.
-
-        Name Collection -> Select Collection -> Select Tabs -> (dismiss fragment) <- Rename Collection
-         */
-
         val tabCollectionCount = store.state.tabCollections.size
         val tabCount = store.state.tabs.size
 

app/src/main/java/org/mozilla/fenix/collections/CollectionCreationFragment.kt

@@ -74,10 +74,10 @@ class CollectionCreationFragment : DialogFragment() {
             DefaultCollectionCreationController(
                 collectionCreationStore,
                 ::dismiss,
-                requireComponents.analytics,
+                requireComponents.analytics.metrics,
                 requireComponents.core.tabCollectionStorage,
                 requireComponents.core.sessionManager,
-                viewLifecycleOwner.lifecycleScope
+                scope = lifecycleScope
             )
         )
         collectionCreationView = CollectionCreationView(

app/src/test/java/org/mozilla/fenix/collections/DefaultCollectionCreationControllerTest.kt

@@ -5,48 +5,167 @@ import io.mockk.every
 import io.mockk.impl.annotations.MockK
 import io.mockk.mockk
 import io.mockk.verify
+import io.mockk.verifyAll
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.TestCoroutineScope
+import kotlinx.coroutines.test.runBlockingTest
 import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
+import mozilla.components.browser.state.state.MediaState
 import mozilla.components.feature.tab.collections.TabCollection
-import mozilla.components.feature.tabs.TabsUseCases
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.junit.Before
 import org.junit.Test
-import org.mozilla.fenix.components.Analytics
 import org.mozilla.fenix.components.TabCollectionStorage
+import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.components.metrics.MetricController
+import org.mozilla.fenix.home.Tab
 
 @ExperimentalCoroutinesApi
 class DefaultCollectionCreationControllerTest {
 
     private val testCoroutineScope = TestCoroutineScope()
-
+    private lateinit var state: CollectionCreationState
     private lateinit var controller: DefaultCollectionCreationController
 
     @MockK(relaxed = true) private lateinit var store: CollectionCreationStore
     @MockK(relaxed = true) private lateinit var dismiss: () -> Unit
-    @MockK(relaxed = true) private lateinit var analytics: Analytics
+    @MockK(relaxUnitFun = true) private lateinit var metrics: MetricController
-    @MockK private lateinit var tabCollectionStorage: TabCollectionStorage
+    @MockK(relaxUnitFun = true) private lateinit var tabCollectionStorage: TabCollectionStorage
-    @MockK private lateinit var tabsUseCases: TabsUseCases
     @MockK private lateinit var sessionManager: SessionManager
-    @MockK private lateinit var state: CollectionCreationState
 
     @Before
     fun before() {
         MockKAnnotations.init(this)
 
-        every { store.state } returns state
+        state = CollectionCreationState(
-        every { state.tabCollections } returns emptyList()
+            tabCollections = emptyList(),
-        every { state.tabs } returns emptyList()
+            tabs = emptyList()
+        )
+        every { store.state } answers { state }
 
         controller = DefaultCollectionCreationController(
-            store, dismiss, analytics,
+            store, dismiss, metrics,
             tabCollectionStorage, sessionManager, testCoroutineScope
         )
     }
 
+    @Test
+    fun `GIVEN tab list WHEN saveCollectionName is called THEN collection should be created`() {
+        val session = mockSession(sessionId = "session-1")
+        val sessions = listOf(
+            session,
+            mockSession(sessionId = "session-2")
+        )
+        every { sessionManager.findSessionById("session-1") } returns session
+        every { sessionManager.findSessionById("null-session") } returns null
+        every { sessionManager.sessions } returns sessions
+        val tabs = listOf(
+            Tab("session-1", "", "", "", mediaState = MediaState.State.NONE),
+            Tab("null-session", "", "", "", mediaState = MediaState.State.NONE)
+        )
+
+        controller.saveCollectionName(tabs, "name")
+
+        verify { dismiss() }
+        verify { tabCollectionStorage.createCollection("name", listOf(session)) }
+        verify { metrics.track(Event.CollectionSaved(2, 1)) }
+    }
+
+    @Test
+    fun `GIVEN name collection WHEN backPressed is called THEN next step should be dispatched`() {
+        state = state.copy(tabCollections = listOf(mockk()))
+        controller.backPressed(SaveCollectionStep.NameCollection)
+        verify { store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.SelectCollection)) }
+
+        state = state.copy(tabCollections = emptyList(), tabs = listOf(mockk(), mockk()))
+        controller.backPressed(SaveCollectionStep.NameCollection)
+        verify { store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.SelectTabs)) }
+
+        state = state.copy(tabCollections = emptyList(), tabs = listOf(mockk()))
+        controller.backPressed(SaveCollectionStep.NameCollection)
+        verify { dismiss() }
+    }
+
+    @Test
+    fun `GIVEN select collection WHEN backPressed is called THEN next step should be dispatched`() {
+        state = state.copy(tabCollections = emptyList(), tabs = listOf(mockk(), mockk()))
+        controller.backPressed(SaveCollectionStep.SelectCollection)
+        verify { store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.SelectTabs)) }
+
+        state = state.copy(tabCollections = emptyList(), tabs = listOf(mockk()))
+        controller.backPressed(SaveCollectionStep.SelectCollection)
+        verify { dismiss() }
+    }
+
+    @Test
+    fun `GIVEN last step WHEN backPressed is called THEN dismiss should be called`() {
+        controller.backPressed(SaveCollectionStep.SelectTabs)
+        verify { dismiss() }
+
+        controller.backPressed(SaveCollectionStep.RenameCollection)
+        verify { dismiss() }
+    }
+
+    @Test
+    fun `GIVEN collection WHEN renameCollection is called THEN collection should be renamed`() = testCoroutineScope.runBlockingTest {
+        val collection = mockk<TabCollection>()
+
+        controller.renameCollection(collection, "name")
+        advanceUntilIdle()
+
+        verifyAll {
+            dismiss()
+            tabCollectionStorage.renameCollection(collection, "name")
+            metrics.track(Event.CollectionRenamed)
+        }
+    }
+
+    @Test
+    fun `WHEN select all is called THEN add all should be dispatched`() {
+        controller.selectAllTabs()
+        verify { store.dispatch(CollectionCreationAction.AddAllTabs) }
+
+        controller.deselectAllTabs()
+        verify { store.dispatch(CollectionCreationAction.RemoveAllTabs) }
+
+        controller.close()
+        verify { dismiss() }
+    }
+
+    @Test
+    fun `WHEN select tab is called THEN add tab should be dispatched`() {
+        val tab = mockk<Tab>()
+
+        controller.addTabToSelection(tab)
+        verify { store.dispatch(CollectionCreationAction.TabAdded(tab)) }
+
+        controller.removeTabFromSelection(tab)
+        verify { store.dispatch(CollectionCreationAction.TabRemoved(tab)) }
+    }
+
+    @Test
+    fun `WHEN selectCollection is called THEN add tabs should be added to collection`() {
+        val session = mockSession(sessionId = "session-1")
+        val sessions = listOf(
+            session,
+            mockSession(sessionId = "session-2")
+        )
+        every { sessionManager.findSessionById("session-1") } returns session
+        every { sessionManager.sessions } returns sessions
+        val tabs = listOf(
+            Tab("session-1", "", "", "", mediaState = MediaState.State.NONE)
+        )
+        val collection = mockk<TabCollection>()
+
+        controller.selectCollection(collection, tabs)
+
+        verify { dismiss() }
+        verify { tabCollectionStorage.addTabsToCollection(collection, listOf(session)) }
+        verify { metrics.track(Event.CollectionTabsAdded(2, 1)) }
+    }
+
     @Test
     fun `GIVEN previous step was SelectTabs or RenameCollection WHEN stepBack is called THEN null should be returned`() {
         assertNull(controller.stepBack(SaveCollectionStep.SelectTabs))
@@ -55,83 +174,79 @@ class DefaultCollectionCreationControllerTest {
 
     @Test
     fun `GIVEN previous step was SelectCollection AND more than one tab is open WHEN stepBack is called THEN SelectTabs should be returned`() {
-        every { state.tabs } returns listOf(mockk(), mockk())
+        state = state.copy(tabs = listOf(mockk(), mockk()))
 
         assertEquals(SaveCollectionStep.SelectTabs, controller.stepBack(SaveCollectionStep.SelectCollection))
     }
 
     @Test
     fun `GIVEN previous step was SelectCollection AND one or fewer tabs are open WHEN stepbback is called THEN null should be returned`() {
-        every { state.tabs } returns listOf(mockk())
+        state = state.copy(tabs = listOf(mockk()))
         assertNull(controller.stepBack(SaveCollectionStep.SelectCollection))
 
-        every { state.tabs } returns emptyList()
+        state = state.copy(tabs = emptyList())
         assertNull(controller.stepBack(SaveCollectionStep.SelectCollection))
     }
 
     @Test
     fun `GIVEN previous step was NameCollection AND tabCollections is empty AND more than one tab is open WHEN stepBack is called THEN SelectTabs should be returned`() {
-        every { state.tabCollections } returns emptyList()
+        state = state.copy(tabCollections = emptyList(), tabs = listOf(mockk(), mockk()))
-        every { state.tabs } returns listOf(mockk(), mockk())
 
         assertEquals(SaveCollectionStep.SelectTabs, controller.stepBack(SaveCollectionStep.NameCollection))
     }
 
     @Test
     fun `GIVEN previous step was NameCollection AND tabCollections is empty AND one or fewer tabs are open WHEN stepBack is called THEN null should be returned`() {
-        every { state.tabCollections } returns emptyList()
+        state = state.copy(tabCollections = emptyList(), tabs = listOf(mockk()))
-        every { state.tabs } returns listOf(mockk())
         assertNull(controller.stepBack(SaveCollectionStep.NameCollection))
 
-        every { state.tabCollections } returns emptyList()
+        state = state.copy(tabCollections = emptyList(), tabs = emptyList())
-        every { state.tabs } returns emptyList()
         assertNull(controller.stepBack(SaveCollectionStep.NameCollection))
     }
 
     @Test
     fun `GIVEN previous step was NameCollection AND tabCollections is not empty WHEN stepBack is called THEN SelectCollection should be returned`() {
-        every { state.tabCollections } returns listOf(mockk())
+        state = state.copy(tabCollections = listOf(mockk()))
-
         assertEquals(SaveCollectionStep.SelectCollection, controller.stepBack(SaveCollectionStep.NameCollection))
     }
 
     @Test
     fun `GIVEN list of collections WHEN default collection number is required THEN return next default number`() {
-        val collections: MutableList<TabCollection> = ArrayList()
+        val collections = mutableListOf<TabCollection>(
-        collections.add(mockk {
+            mockk {
-            every { title } returns "Collection 1"
+                every { title } returns "Collection 1"
-        })
+            },
-        collections.add(mockk {
+            mockk {
-            every { title } returns "Collection 2"
+                every { title } returns "Collection 2"
-        })
+            },
-        collections.add(mockk {
+            mockk {
-            every { title } returns "Collection 3"
+                every { title } returns "Collection 3"
-        })
+            }
-        every { state.tabCollections } returns collections
+        )
-
+        state = state.copy(tabCollections = collections)
         assertEquals(4, controller.getDefaultCollectionNumber())
 
         collections.add(mockk {
             every { title } returns "Collection 5"
         })
+        state = state.copy(tabCollections = collections)
         assertEquals(6, controller.getDefaultCollectionNumber())
 
         collections.add(mockk {
             every { title } returns "Random name"
         })
+        state = state.copy(tabCollections = collections)
         assertEquals(6, controller.getDefaultCollectionNumber())
 
         collections.add(mockk {
             every { title } returns "Collection 10 10"
         })
+        state = state.copy(tabCollections = collections)
         assertEquals(6, controller.getDefaultCollectionNumber())
     }
 
     @Test
     fun `WHEN adding a new collection THEN dispatch NameCollection step changed`() {
-        val collections: List<TabCollection> = ArrayList()
-        every { state.tabCollections } returns collections
-
         controller.addNewCollection()
 
         verify { store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.NameCollection, 1)) }
@@ -139,9 +254,6 @@ class DefaultCollectionCreationControllerTest {
 
     @Test
     fun `GIVEN empty list of collections WHEN saving tabs to collection THEN dispatch NameCollection step changed`() {
-        val collections: List<TabCollection> = ArrayList()
-        every { state.tabCollections } returns collections
-
         controller.saveTabsToCollection(ArrayList())
 
         verify { store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.NameCollection, 1)) }
@@ -149,14 +261,14 @@ class DefaultCollectionCreationControllerTest {
 
     @Test
     fun `GIVEN list of collections WHEN saving tabs to collection THEN dispatch NameCollection step changed`() {
-        val collections: MutableList<TabCollection> = ArrayList()
+        state = state.copy(tabCollections = listOf(
-        collections.add(mockk {
+            mockk {
-            every { title } returns "Collection 1"
+                every { title } returns "Collection 1"
-        })
+            },
-        collections.add(mockk {
+            mockk {
-            every { title } returns "Random Collection"
+                every { title } returns "Random Collection"
-        })
+            }
-        every { state.tabCollections } returns collections
+        ))
 
         controller.saveTabsToCollection(ArrayList())
 
@@ -165,27 +277,32 @@ class DefaultCollectionCreationControllerTest {
 
     @Test
     fun `normalSessionSize only counts non-private non-custom sessions`() {
-        fun session(isPrivate: Boolean, isCustom: Boolean) = mockk<Session>().apply {
+        val normal1 = mockSession()
-            every { private } returns isPrivate
+        val normal2 = mockSession()
-            every { isCustomTabSession() } returns isCustom
+        val normal3 = mockSession()
-        }
 
-        val normal1 = session(isPrivate = false, isCustom = false)
+        val private1 = mockSession(isPrivate = true)
-        val normal2 = session(isPrivate = false, isCustom = false)
+        val private2 = mockSession(isPrivate = true)
-        val normal3 = session(isPrivate = false, isCustom = false)
 
-        val private1 = session(isPrivate = true, isCustom = false)
+        val custom1 = mockSession(isCustom = true)
-        val private2 = session(isPrivate = true, isCustom = false)
+        val custom2 = mockSession(isCustom = true)
+        val custom3 = mockSession(isCustom = true)
 
-        val custom1 = session(isPrivate = false, isCustom = true)
+        val privateCustom = mockSession(isPrivate = true, isCustom = true)
-        val custom2 = session(isPrivate = false, isCustom = true)
-        val custom3 = session(isPrivate = false, isCustom = true)
-
-        val privateCustom = session(isPrivate = true, isCustom = true)
 
         every { sessionManager.sessions } returns listOf(normal1, private1, private2, custom1,
             normal2, normal3, custom2, custom3, privateCustom)
 
         assertEquals(3, controller.normalSessionSize(sessionManager))
     }
+
+    private fun mockSession(
+        sessionId: String? = null,
+        isPrivate: Boolean = false,
+        isCustom: Boolean = false
+    ) = mockk<Session> {
+        sessionId?.let { every { id } returns it }
+        every { private } returns isPrivate
+        every { isCustomTabSession() } returns isCustom
+    }
 }