diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt index 35635caf1..e3c1a7310 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt @@ -29,7 +29,7 @@ import mozilla.components.support.ktx.kotlin.isExtensionUrl import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav import org.mozilla.fenix.share.listadapters.AppShareOption @@ -66,7 +66,6 @@ interface ShareController { @Suppress("TooManyFunctions") class DefaultShareController( private val context: Context, - private val metrics: MetricController, private val shareSubject: String?, private val shareData: List, private val sendTabUseCases: SendTabUseCases, @@ -123,7 +122,7 @@ class DefaultShareController( } override fun handleShareToDevice(device: Device) { - metrics.track(Event.SendTab) + context.metrics.track(Event.SendTab) shareToDevicesWithRetry { sendTabUseCases.sendToDeviceAsync(device.id, shareData.toTabData()) } } @@ -132,7 +131,7 @@ class DefaultShareController( } override fun handleSignIn() { - metrics.track(Event.SignInToSendTab) + context.metrics.track(Event.SignInToSendTab) val directions = ShareFragmentDirections.actionGlobalTurnOnSync(padSnackbar = true) navController.nav(R.id.shareFragment, directions) diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt b/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt index 0a4ed5ae0..cd190d564 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt @@ -67,7 +67,6 @@ class ShareFragment : AppCompatDialogFragment() { shareInteractor = ShareInteractor( DefaultShareController( context = requireContext(), - metrics = requireComponents.analytics.metrics, shareSubject = args.shareSubject, shareData = shareData, snackbar = FenixSnackbar.make( diff --git a/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt b/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt index 9950dd5b6..90540b24c 100644 --- a/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt @@ -10,13 +10,12 @@ import android.content.Context import android.content.Intent import androidx.navigation.NavController import com.google.android.material.snackbar.Snackbar -import io.mockk.MockKAnnotations import io.mockk.Runs import io.mockk.every -import io.mockk.impl.annotations.MockK import io.mockk.just import io.mockk.mockk import io.mockk.slot +import io.mockk.spyk import io.mockk.verify import io.mockk.verifyOrder import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -39,6 +38,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.share.listadapters.AppShareOption @@ -46,15 +46,9 @@ import org.mozilla.fenix.share.listadapters.AppShareOption @RunWith(FenixRobolectricTestRunner::class) @ExperimentalCoroutinesApi class ShareControllerTest { - - @MockK(relaxed = true) private lateinit var metrics: MetricController - @MockK(relaxed = true) private lateinit var sendTabUseCases: SendTabUseCases - @MockK(relaxed = true) private lateinit var snackbar: FenixSnackbar - @MockK(relaxed = true) private lateinit var navController: NavController - @MockK(relaxed = true) private lateinit var dismiss: (ShareController.Result) -> Unit - @MockK private lateinit var recentAppStorage: RecentAppsStorage - - private val context: Context = testContext + // Need a valid context to retrieve Strings for example, but we also need it to return our "metrics" + private val context: Context = spyk(testContext) + private val metrics: MetricController = mockk(relaxed = true) private val shareSubject = "shareSubject" private val shareData = listOf( ShareData(url = "url0", title = "title0"), @@ -67,15 +61,23 @@ class ShareControllerTest { ) private val textToShare = "${shareData[0].url}\n\n${shareData[1].url}" private val testCoroutineScope = TestCoroutineScope() + private val sendTabUseCases = mockk(relaxed = true) + private val snackbar = mockk(relaxed = true) + private val navController = mockk(relaxed = true) + private val dismiss = mockk<(ShareController.Result) -> Unit>(relaxed = true) + private val recentAppStorage = mockk(relaxed = true) + private val controller = DefaultShareController( + context, shareSubject, shareData, sendTabUseCases, snackbar, navController, + recentAppStorage, testCoroutineScope, dismiss + ) @Before fun setUp() { - MockKAnnotations.init(this) + every { context.metrics } returns metrics } @Test fun `handleShareClosed should call a passed in delegate to close this`() { - val controller = buildController() controller.handleShareClosed() verify { dismiss(ShareController.Result.DISMISSED) } @@ -83,20 +85,17 @@ class ShareControllerTest { @Test fun `handleShareToApp should start a new sharing activity and close this`() = runBlocking { - val appShareOption = AppShareOption( - name = "app", - icon = mockk(), - packageName = "package", - activityName = "activity" - ) + val appPackageName = "package" + val appClassName = "activity" + val appShareOption = AppShareOption("app", mockk(), appPackageName, appClassName) val shareIntent = slot() // Our share Intent uses `FLAG_ACTIVITY_NEW_TASK` but when resolving the startActivity call // needed for capturing the actual Intent used the `slot` one doesn't have this flag so we // need to use an Activity Context. - val activityContext: Context = mockk { - every { startActivity(capture(shareIntent)) } just Runs - } - val testController = buildController(context = activityContext) + val activityContext: Context = mockk() + val testController = DefaultShareController(activityContext, shareSubject, shareData, mockk(), + mockk(), mockk(), recentAppStorage, testCoroutineScope, dismiss) + every { activityContext.startActivity(capture(shareIntent)) } just Runs every { recentAppStorage.updateRecentApp(appShareOption.activityName) } just Runs testController.handleShareToApp(appShareOption) @@ -108,8 +107,8 @@ class ShareControllerTest { assertEquals(textToShare, shareIntent.captured.extras!![Intent.EXTRA_TEXT]) assertEquals("text/plain", shareIntent.captured.type) assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK, shareIntent.captured.flags) - assertEquals("package", shareIntent.captured.component!!.packageName) - assertEquals("activity", shareIntent.captured.component!!.className) + assertEquals(appPackageName, shareIntent.captured.component!!.packageName) + assertEquals(appClassName, shareIntent.captured.component!!.className) verify { recentAppStorage.updateRecentApp(appShareOption.activityName) } verifyOrder { @@ -120,21 +119,18 @@ class ShareControllerTest { @Test fun `handleShareToApp should dismiss with an error start when a security exception occurs`() { - val appShareOption = AppShareOption( - name = "app", - icon = mockk(), - packageName = "package", - activityName = "activity" - ) + val appPackageName = "package" + val appClassName = "activity" + val appShareOption = AppShareOption("app", mockk(), appPackageName, appClassName) val shareIntent = slot() // Our share Intent uses `FLAG_ACTIVITY_NEW_TASK` but when resolving the startActivity call // needed for capturing the actual Intent used the `slot` one doesn't have this flag so we // need to use an Activity Context. - val activityContext: Context = mockk { - every { startActivity(capture(shareIntent)) } throws SecurityException() - every { getString(R.string.share_error_snackbar) } returns "Cannot share to this app" - } - val testController = buildController(context = activityContext) + val activityContext: Context = mockk() + val testController = DefaultShareController(activityContext, shareSubject, shareData, mockk(), + snackbar, mockk(), mockk(), testCoroutineScope, dismiss) + every { activityContext.startActivity(capture(shareIntent)) } throws SecurityException() + every { activityContext.getString(R.string.share_error_snackbar) } returns "Cannot share to this app" testController.handleShareToApp(appShareOption) @@ -179,7 +175,6 @@ class ShareControllerTest { val deviceId = slot() val tabsShared = slot>() - val controller = buildController() controller.handleShareToDevice(deviceToShareTo) // Verify all the needed methods are called. @@ -204,7 +199,6 @@ class ShareControllerTest { ) val tabsShared = slot>() - val controller = buildController() controller.handleShareToAllDevices(devicesToShareTo) verifyOrder { @@ -219,7 +213,6 @@ class ShareControllerTest { @Test fun `handleSignIn should navigate to the Sync Fragment and dismiss this one`() { - val controller = buildController() controller.handleSignIn() verifyOrder { @@ -234,7 +227,6 @@ class ShareControllerTest { @Test fun `handleReauth should navigate to the Account Problem Fragment and dismiss this one`() { - val controller = buildController() controller.handleReauth() verifyOrder { @@ -248,7 +240,6 @@ class ShareControllerTest { @Test fun `showSuccess should show a snackbar with a success message`() { - val controller = buildController() val expectedMessage = controller.getSuccessMessage() val expectedTimeout = Snackbar.LENGTH_SHORT @@ -268,7 +259,7 @@ class ShareControllerTest { val expectedRetryMessage = context.getString(R.string.sync_sent_tab_error_snackbar_action) - buildController().showFailureWithRetryOption(operation) + controller.showFailureWithRetryOption(operation) verify { snackbar.apply { @@ -282,12 +273,18 @@ class ShareControllerTest { @Test fun `getSuccessMessage should return different strings depending on the number of shared tabs`() { - val controllerWithOneSharedTab = buildController( - data = listOf(ShareData(url = "url0", title = "title0")) - ) - val controllerWithMoreSharedTabs = buildController( - data = shareData + val controllerWithOneSharedTab = DefaultShareController( + context, + shareSubject, + listOf(ShareData(url = "url0", title = "title0")), + mockk(), + mockk(), + mockk(), + mockk(), + mockk(), + mockk() ) + val controllerWithMoreSharedTabs = controller val expectedTabSharedMessage = context.getString(R.string.sync_sent_tab_snackbar) val expectedTabsSharedMessage = context.getString(R.string.sync_sent_tabs_snackbar) @@ -301,7 +298,7 @@ class ShareControllerTest { @Test fun `getShareText should respect concatenate shared tabs urls`() { - assertEquals(textToShare, buildController(data = shareData).getShareText()) + assertEquals(textToShare, controller.getShareText()) } @Test @@ -311,7 +308,10 @@ class ShareControllerTest { ShareData(url = "moz-extension://eb8df45a-895b-4f3a-896a-c0c71ae5/page.html?url=url0"), ShareData(url = "url1") ) - val controller = buildController(data = shareData) + val controller = DefaultShareController( + context, shareSubject, shareData, sendTabUseCases, snackbar, navController, + recentAppStorage, testCoroutineScope, dismiss + ) val expectedShareText = "${shareData[0].url}\n\nurl0\n\n${shareData[2].url}" assertEquals(expectedShareText, controller.getShareText()) @@ -319,20 +319,25 @@ class ShareControllerTest { @Test fun `getShareSubject will return "shareSubject" if that is non null`() { - assertEquals(shareSubject, buildController(subject = shareSubject).getShareSubject()) + assertEquals(shareSubject, controller.getShareSubject()) } @Test fun `getShareSubject will return a concatenation of tab titles if "shareSubject" is null`() { - val controller = buildController(subject = null) + val controller = DefaultShareController( + context, null, shareData, sendTabUseCases, snackbar, navController, + recentAppStorage, testCoroutineScope, dismiss + ) assertEquals("title0, title1", controller.getShareSubject()) } @Test fun `ShareTab#toTabData maps a list of ShareTab to a TabData list`() { - val tabData = with(buildController()) { - shareData.toTabData() + var tabData: List + + with(controller) { + tabData = shareData.toTabData() } assertEquals(tabsData, tabData) @@ -340,13 +345,14 @@ class ShareControllerTest { @Test fun `ShareTab#toTabData creates a data url from text if no url is specified`() { + var tabData: List val expected = listOf( TabData(title = "title0", url = ""), TabData(title = "title1", url = "data:,Hello%2C%20World!") ) - val tabData = with(buildController()) { - listOf( + with(controller) { + tabData = listOf( ShareData(title = "title0"), ShareData(title = "title1", text = "Hello, World!") ).toTabData() @@ -354,21 +360,4 @@ class ShareControllerTest { assertEquals(expected, tabData) } - - private fun buildController( - context: Context = this.context, - subject: String? = shareSubject, - data: List = shareData - ) = DefaultShareController( - context = context, - metrics = metrics, - shareSubject = subject, - shareData = data, - sendTabUseCases = sendTabUseCases, - snackbar = snackbar, - navController = navController, - recentAppsStorage = recentAppStorage, - viewLifecycleScope = testCoroutineScope, - dismiss = dismiss - ) }