Revert "For #12565: Pass metrics to share controller" for debug test failures.

This reverts commit bbaca06274.
pull/184/head
Sebastian Kaspari 4 years ago
parent 4de466883b
commit 2fda22e857

@ -29,7 +29,7 @@ import mozilla.components.support.ktx.kotlin.isExtensionUrl
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.metrics.Event 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.ext.nav
import org.mozilla.fenix.share.listadapters.AppShareOption import org.mozilla.fenix.share.listadapters.AppShareOption
@ -66,7 +66,6 @@ interface ShareController {
@Suppress("TooManyFunctions") @Suppress("TooManyFunctions")
class DefaultShareController( class DefaultShareController(
private val context: Context, private val context: Context,
private val metrics: MetricController,
private val shareSubject: String?, private val shareSubject: String?,
private val shareData: List<ShareData>, private val shareData: List<ShareData>,
private val sendTabUseCases: SendTabUseCases, private val sendTabUseCases: SendTabUseCases,
@ -123,7 +122,7 @@ class DefaultShareController(
} }
override fun handleShareToDevice(device: Device) { override fun handleShareToDevice(device: Device) {
metrics.track(Event.SendTab) context.metrics.track(Event.SendTab)
shareToDevicesWithRetry { sendTabUseCases.sendToDeviceAsync(device.id, shareData.toTabData()) } shareToDevicesWithRetry { sendTabUseCases.sendToDeviceAsync(device.id, shareData.toTabData()) }
} }
@ -132,7 +131,7 @@ class DefaultShareController(
} }
override fun handleSignIn() { override fun handleSignIn() {
metrics.track(Event.SignInToSendTab) context.metrics.track(Event.SignInToSendTab)
val directions = val directions =
ShareFragmentDirections.actionGlobalTurnOnSync(padSnackbar = true) ShareFragmentDirections.actionGlobalTurnOnSync(padSnackbar = true)
navController.nav(R.id.shareFragment, directions) navController.nav(R.id.shareFragment, directions)

@ -67,7 +67,6 @@ class ShareFragment : AppCompatDialogFragment() {
shareInteractor = ShareInteractor( shareInteractor = ShareInteractor(
DefaultShareController( DefaultShareController(
context = requireContext(), context = requireContext(),
metrics = requireComponents.analytics.metrics,
shareSubject = args.shareSubject, shareSubject = args.shareSubject,
shareData = shareData, shareData = shareData,
snackbar = FenixSnackbar.make( snackbar = FenixSnackbar.make(

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

Loading…
Cancel
Save