From a5716396bee28d3931c1ce04e7b8568281f3b47b Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Thu, 29 Jul 2021 16:04:07 -0400 Subject: [PATCH] [fenix] Issue https://github.com/mozilla-mobile/fenix/issues/20533: Fix intermittent failures of DefaultTabsTrayControllerTest --- .../tabstray/DefaultTabsTrayControllerTest.kt | 187 +++++++++--------- 1 file changed, 91 insertions(+), 96 deletions(-) diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt index 67cc7409b9..f1875b9ecd 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt @@ -24,6 +24,9 @@ import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.base.profiler.Profiler import mozilla.components.concept.tabstray.Tab import mozilla.components.feature.tabs.TabsUseCases +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -48,9 +51,6 @@ class DefaultTabsTrayControllerTest { @MockK(relaxed = true) private lateinit var navController: NavController - @MockK(relaxed = true) - private lateinit var navigateToHomeAndDeleteSession: (String) -> Unit - @MockK(relaxed = true) private lateinit var profiler: Profiler @@ -63,34 +63,9 @@ class DefaultTabsTrayControllerTest { @MockK(relaxed = true) private lateinit var tabsUseCases: TabsUseCases - @MockK(relaxed = true) - private lateinit var selectTabPosition: (Int, Boolean) -> Unit - - @MockK(relaxed = true) - private lateinit var dismissTray: () -> Unit - - @MockK(relaxed = true) - private lateinit var showUndoSnackbarForTab: (Boolean) -> Unit - - private lateinit var controller: DefaultTabsTrayController - @Before fun setup() { MockKAnnotations.init(this) - controller = DefaultTabsTrayController( - trayStore, - browserStore, - browsingModeManager, - navController, - navigateToHomeAndDeleteSession, - profiler, - navigationInteractor, - metrics, - tabsUseCases, - selectTabPosition, - dismissTray, - showUndoSnackbarForTab - ) } @Test @@ -98,22 +73,8 @@ class DefaultTabsTrayControllerTest { profiler = spyk(profiler) { every { getProfilerTime() } returns Double.MAX_VALUE } - controller = DefaultTabsTrayController( - trayStore, - browserStore, - browsingModeManager, - navController, - navigateToHomeAndDeleteSession, - profiler, - navigationInteractor, - metrics, - tabsUseCases, - selectTabPosition, - dismissTray, - showUndoSnackbarForTab - ) - controller.handleOpeningNewTab(true) + createController().handleOpeningNewTab(true) verifyOrder { profiler.getProfilerTime() @@ -133,22 +94,8 @@ class DefaultTabsTrayControllerTest { profiler = spyk(profiler) { every { getProfilerTime() } returns Double.MAX_VALUE } - controller = DefaultTabsTrayController( - trayStore, - browserStore, - browsingModeManager, - navController, - navigateToHomeAndDeleteSession, - profiler, - navigationInteractor, - metrics, - tabsUseCases, - selectTabPosition, - dismissTray, - showUndoSnackbarForTab - ) - controller.handleOpeningNewTab(false) + createController().handleOpeningNewTab(false) verifyOrder { profiler.getProfilerTime() @@ -165,42 +112,40 @@ class DefaultTabsTrayControllerTest { @Test fun `GIVEN private mode WHEN handleOpeningNewTab is called THEN Event#NewPrivateTabTapped is added to telemetry`() { - controller.handleOpeningNewTab(true) + createController().handleOpeningNewTab(true) verify { metrics.track(Event.NewPrivateTabTapped) } } @Test fun `GIVEN private mode WHEN handleOpeningNewTab is called THEN Event#NewTabTapped is added to telemetry`() { - controller.handleOpeningNewTab(false) + createController().handleOpeningNewTab(false) verify { metrics.track(Event.NewTabTapped) } } @Test fun `WHEN handleTrayScrollingToPosition is called with smoothScroll=true THEN it scrolls to that position with smoothScroll`() { - controller.handleTrayScrollingToPosition(3, true) + var selectTabPositionInvoked = false + createController(selectTabPosition = { position, smoothScroll -> + assertEquals(3, position) + assertTrue(smoothScroll) + selectTabPositionInvoked = true + }).handleTrayScrollingToPosition(3, true) - verify { selectTabPosition(3, true) } + assertTrue(selectTabPositionInvoked) } @Test fun `WHEN handleTrayScrollingToPosition is called with smoothScroll=true THEN it emits an action for the tray page of that tab position`() { - controller.handleTrayScrollingToPosition(33, true) + createController().handleTrayScrollingToPosition(33, true) verify { trayStore.dispatch(TabsTrayAction.PageSelected(Page.positionToPage(33))) } } - @Test - fun `WHEN handleTrayScrollingToPosition is called with smoothScroll=false THEN it scrolls to that position without smoothScroll`() { - controller.handleTrayScrollingToPosition(4, false) - - verify { selectTabPosition(4, false) } - } - @Test fun `WHEN handleTrayScrollingToPosition is called with smoothScroll=false THEN it emits an action for the tray page of that tab position`() { - controller.handleTrayScrollingToPosition(44, true) + createController().handleTrayScrollingToPosition(44, true) verify { trayStore.dispatch(TabsTrayAction.PageSelected(Page.positionToPage(44))) } } @@ -209,9 +154,10 @@ class DefaultTabsTrayControllerTest { fun `GIVEN already on browserFragment WHEN handleNavigateToBrowser is called THEN the tray is dismissed`() { every { navController.currentDestination?.id } returns R.id.browserFragment - controller.handleNavigateToBrowser() + var dismissTrayInvoked = false + createController(dismissTray = { dismissTrayInvoked = true }).handleNavigateToBrowser() - verify { dismissTray() } + assertTrue(dismissTrayInvoked) verify(exactly = 0) { navController.popBackStack() } verify(exactly = 0) { navController.popBackStack(any(), any()) } verify(exactly = 0) { navController.navigate(any()) } @@ -224,9 +170,10 @@ class DefaultTabsTrayControllerTest { every { navController.currentDestination?.id } returns R.id.browserFragment + 1 every { navController.popBackStack(R.id.browserFragment, false) } returns true - controller.handleNavigateToBrowser() + var dismissTrayInvoked = false + createController(dismissTray = { dismissTrayInvoked = true }).handleNavigateToBrowser() - verify { dismissTray() } + assertTrue(dismissTrayInvoked) verify { navController.popBackStack(R.id.browserFragment, false) } verify(exactly = 0) { navController.navigate(any()) } verify(exactly = 0) { navController.navigate(any()) } @@ -238,9 +185,10 @@ class DefaultTabsTrayControllerTest { every { navController.currentDestination?.id } returns R.id.browserFragment + 1 every { navController.popBackStack(R.id.browserFragment, false) } returns false - controller.handleNavigateToBrowser() + var dismissTrayInvoked = false + createController(dismissTray = { dismissTrayInvoked = true }).handleNavigateToBrowser() - verify { dismissTray() } + assertTrue(dismissTrayInvoked) verify { navController.popBackStack(R.id.browserFragment, false) } verify { navController.navigate(R.id.browserFragment) } } @@ -249,9 +197,10 @@ class DefaultTabsTrayControllerTest { fun `GIVEN not already on browserFragment WHEN handleNavigateToBrowser is called and popBackStack succeeds THEN the method finishes`() { every { navController.popBackStack(R.id.browserFragment, false) } returns true - controller.handleNavigateToBrowser() + var dismissTrayInvoked = false + createController(dismissTray = { dismissTrayInvoked = true }).handleNavigateToBrowser() - verify { dismissTray() } + assertTrue(dismissTrayInvoked) verify(exactly = 1) { navController.popBackStack(R.id.browserFragment, false) } verify(exactly = 0) { navController.navigate(R.id.browserFragment) } } @@ -268,10 +217,14 @@ class DefaultTabsTrayControllerTest { every { browserStore.state.findTab(any()) } returns tab every { browserStore.state.getNormalOrPrivateTabs(any()) } returns listOf(tab, mockk()) - controller.handleTabDeletion("22") + var showUndoSnackbarForTabInvoked = false + createController(showUndoSnackbarForTab = { + assertTrue(it) + showUndoSnackbarForTabInvoked = true + }).handleTabDeletion("22") verify { tabsUseCases.removeTab("22") } - verify { showUndoSnackbarForTab(true) } + assertTrue(showUndoSnackbarForTabInvoked) } finally { unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt") } @@ -279,7 +232,8 @@ class DefaultTabsTrayControllerTest { @Test fun `GIVEN only one tab opened WHEN handleTabDeletion is called THEN that it navigates to home where the tab will be removed`() { - controller = spyk(controller) + var showUndoSnackbarForTabInvoked = false + val controller = spyk(createController(showUndoSnackbarForTab = { showUndoSnackbarForTabInvoked = true })) val tab: TabSessionState = mockk { every { content } returns mockk() every { content.private } returns true @@ -294,7 +248,7 @@ class DefaultTabsTrayControllerTest { verify { controller.dismissTabsTrayAndNavigateHome("33") } verify(exactly = 0) { tabsUseCases.removeTab(any()) } - verify(exactly = 0) { showUndoSnackbarForTab(any()) } + assertFalse(showUndoSnackbarForTabInvoked) } finally { unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt") } @@ -303,7 +257,12 @@ class DefaultTabsTrayControllerTest { @ExperimentalCoroutinesApi @Test fun `WHEN handleMultipleTabsDeletion is called to close all private tabs THEN that it navigates to home where that tabs will be removed and shows undo snackbar`() { - controller = spyk(controller) + var showUndoSnackbarForTabInvoked = false + val controller = spyk(createController(showUndoSnackbarForTab = { + assertTrue(it) + showUndoSnackbarForTabInvoked = true + })) + val privateTab: Tab = mockk { every { private } returns true } @@ -315,7 +274,7 @@ class DefaultTabsTrayControllerTest { controller.handleMultipleTabsDeletion(listOf(privateTab, mockk())) verify { controller.dismissTabsTrayAndNavigateHome(HomeFragment.ALL_PRIVATE_TABS) } - verify { showUndoSnackbarForTab(true) } + assertTrue(showUndoSnackbarForTabInvoked) verify(exactly = 0) { tabsUseCases.removeTabs(any()) } } finally { unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt") @@ -325,7 +284,11 @@ class DefaultTabsTrayControllerTest { @ExperimentalCoroutinesApi @Test fun `WHEN handleMultipleTabsDeletion is called to close all normal tabs THEN that it navigates to home where that tabs will be removed and shows undo snackbar`() { - controller = spyk(controller) + var showUndoSnackbarForTabInvoked = false + val controller = spyk(createController(showUndoSnackbarForTab = { + assertFalse(it) + showUndoSnackbarForTabInvoked = true + })) val normalTab: Tab = mockk { every { private } returns false } @@ -337,8 +300,8 @@ class DefaultTabsTrayControllerTest { controller.handleMultipleTabsDeletion(listOf(normalTab, normalTab)) verify { controller.dismissTabsTrayAndNavigateHome(HomeFragment.ALL_NORMAL_TABS) } - verify { showUndoSnackbarForTab(false) } verify(exactly = 0) { tabsUseCases.removeTabs(any()) } + assertTrue(showUndoSnackbarForTabInvoked) } finally { unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt") } @@ -347,7 +310,8 @@ class DefaultTabsTrayControllerTest { @ExperimentalCoroutinesApi @Test fun `WHEN handleMultipleTabsDeletion is called to close some private tabs THEN that it uses tabsUseCases#removeTabs and shows an undo snackbar`() { - controller = spyk(controller) + var showUndoSnackbarForTabInvoked = false + val controller = spyk(createController(showUndoSnackbarForTab = { showUndoSnackbarForTabInvoked = true })) val privateTab: Tab = mockk { every { private } returns true every { id } returns "42" @@ -360,8 +324,8 @@ class DefaultTabsTrayControllerTest { controller.handleMultipleTabsDeletion(listOf(privateTab)) verify { tabsUseCases.removeTabs(listOf("42")) } - verify { showUndoSnackbarForTab(true) } verify(exactly = 0) { controller.dismissTabsTrayAndNavigateHome(any()) } + assertTrue(showUndoSnackbarForTabInvoked) } finally { unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt") } @@ -370,7 +334,8 @@ class DefaultTabsTrayControllerTest { @ExperimentalCoroutinesApi @Test fun `WHEN handleMultipleTabsDeletion is called to close some normal tabs THEN that it uses tabsUseCases#removeTabs and shows an undo snackbar`() { - controller = spyk(controller) + var showUndoSnackbarForTabInvoked = false + val controller = spyk(createController(showUndoSnackbarForTab = { showUndoSnackbarForTabInvoked = true })) val privateTab: Tab = mockk { every { private } returns false every { id } returns "24" @@ -383,8 +348,8 @@ class DefaultTabsTrayControllerTest { controller.handleMultipleTabsDeletion(listOf(privateTab)) verify { tabsUseCases.removeTabs(listOf("24")) } - verify { showUndoSnackbarForTab(false) } verify(exactly = 0) { controller.dismissTabsTrayAndNavigateHome(any()) } + assertTrue(showUndoSnackbarForTabInvoked) } finally { unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt") } @@ -392,23 +357,53 @@ class DefaultTabsTrayControllerTest { @Test fun `GIVEN private mode selected WHEN sendNewTabEvent is called THEN NewPrivateTabTapped is tracked in telemetry`() { - controller.sendNewTabEvent(true) + createController().sendNewTabEvent(true) verify { metrics.track(Event.NewPrivateTabTapped) } } @Test fun `GIVEN normal mode selected WHEN sendNewTabEvent is called THEN NewTabTapped is tracked in telemetry`() { - controller.sendNewTabEvent(false) + createController().sendNewTabEvent(false) verify { metrics.track(Event.NewTabTapped) } } @Test fun `WHEN dismissTabsTrayAndNavigateHome is called with a spefic tab id THEN tray is dismissed and navigates home is opened to delete that tab`() { - controller.dismissTabsTrayAndNavigateHome("randomId") + var dismissTrayInvoked = false + var navigateToHomeAndDeleteSessionInvoked = false + createController(dismissTray = { + dismissTrayInvoked = true + }, navigateToHomeAndDeleteSession = { + assertEquals("randomId", it) + navigateToHomeAndDeleteSessionInvoked = true + } + ).dismissTabsTrayAndNavigateHome("randomId") - verify { dismissTray() } - verify { navigateToHomeAndDeleteSession("randomId") } + assertTrue(dismissTrayInvoked) + assertTrue(navigateToHomeAndDeleteSessionInvoked) + } + + private fun createController( + navigateToHomeAndDeleteSession: (String) -> Unit = { }, + selectTabPosition: (Int, Boolean) -> Unit = { _, _ -> }, + dismissTray: () -> Unit = { }, + showUndoSnackbarForTab: (Boolean) -> Unit = { _ -> } + ): DefaultTabsTrayController { + return DefaultTabsTrayController( + trayStore, + browserStore, + browsingModeManager, + navController, + navigateToHomeAndDeleteSession, + profiler, + navigationInteractor, + metrics, + tabsUseCases, + selectTabPosition, + dismissTray, + showUndoSnackbarForTab + ) } }