Skip to content

Commit

Permalink
Revert "For mozilla-mobile#12565: Pass bookmark storage to controller…
Browse files Browse the repository at this point in the history
…" for debug test failures.

This reverts commit 3c22100.
  • Loading branch information
pocmo committed Sep 29, 2020
1 parent cfbad1d commit 4de4668
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import kotlinx.coroutines.launch
import mozilla.appservices.places.BookmarkRoot
import mozilla.components.concept.engine.prompt.ShareData
import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarksStorage
import mozilla.components.service.fxa.manager.FxaAccountManager
import mozilla.components.service.fxa.sync.SyncReason
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.bookmarkStorage
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav

/**
Expand Down Expand Up @@ -52,8 +52,6 @@ interface BookmarkController {
@Suppress("TooManyFunctions")
class DefaultBookmarkController(
private val activity: HomeActivity,
private val bookmarkStorage: BookmarksStorage,
private val accountManager: FxaAccountManager,
private val navController: NavController,
private val clipboardManager: ClipboardManager?,
private val scope: CoroutineScope,
Expand Down Expand Up @@ -145,14 +143,14 @@ class DefaultBookmarkController(
scope.launch {
store.dispatch(BookmarkFragmentAction.StartSync)
invokePendingDeletion()
accountManager.syncNow(SyncReason.User)
activity.components.backgroundServices.accountManager.syncNow(SyncReason.User)
// The current bookmark node we are viewing may be made invalid after syncing so we
// check if the current node is valid and if it isn't we find the nearest valid ancestor
// and open it
val validAncestorGuid = store.state.guidBackstack.findLast { guid ->
bookmarkStorage.getBookmark(guid) != null
activity.bookmarkStorage.getBookmark(guid) != null
} ?: BookmarkRoot.Mobile.id
val node = bookmarkStorage.getBookmark(validAncestorGuid)!!
val node = activity.bookmarkStorage.getBookmark(validAncestorGuid)!!
handleBookmarkExpand(node)
store.dispatch(BookmarkFragmentAction.FinishSync)
}
Expand All @@ -162,12 +160,12 @@ class DefaultBookmarkController(
invokePendingDeletion.invoke()
scope.launch {
val parentGuid = store.state.guidBackstack.findLast { guid ->
store.state.tree?.guid != guid && bookmarkStorage.getBookmark(guid) != null
store.state.tree?.guid != guid && activity.bookmarkStorage.getBookmark(guid) != null
}
if (parentGuid == null) {
navController.popBackStack()
} else {
val parent = bookmarkStorage.getBookmark(parentGuid)!!
val parent = activity.bookmarkStorage.getBookmark(parentGuid)!!
handleBookmarkExpand(parent)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
_bookmarkInteractor = BookmarkFragmentInteractor(
bookmarksController = DefaultBookmarkController(
activity = requireActivity() as HomeActivity,
bookmarkStorage = requireComponents.core.bookmarksStorage,
accountManager = requireComponents.backgroundServices.accountManager,
navController = findNavController(),
clipboardManager = requireContext().getSystemService(),
scope = viewLifecycleOwner.lifecycleScope,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,27 @@ package org.mozilla.fenix.library.bookmarks

import android.content.ClipData
import android.content.ClipboardManager
import android.content.Context
import androidx.navigation.NavController
import androidx.navigation.NavDestination
import androidx.navigation.NavDirections
import io.mockk.MockKAnnotations
import io.mockk.Runs
import io.mockk.called
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.just
import io.mockk.mockk
import io.mockk.runs
import io.mockk.slot
import io.mockk.spyk
import io.mockk.verify
import io.mockk.verifyOrder
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestCoroutineScope
import mozilla.appservices.places.BookmarkRoot
import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.concept.storage.BookmarksStorage
import mozilla.components.service.fxa.manager.FxaAccountManager
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Before
Expand All @@ -34,28 +35,32 @@ import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.Services
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.bookmarkStorage
import org.mozilla.fenix.ext.components

@Suppress("TooManyFunctions", "LargeClass")
@ExperimentalCoroutinesApi
class BookmarkControllerTest {

private val scope = TestCoroutineScope()

@MockK private lateinit var bookmarkStore: BookmarkFragmentStore
@MockK private lateinit var sharedViewModel: BookmarksSharedViewModel
@MockK(relaxUnitFun = true) private lateinit var clipboardManager: ClipboardManager
@MockK(relaxed = true) private lateinit var homeActivity: HomeActivity
@MockK(relaxed = true) private lateinit var bookmarkStorage: BookmarksStorage
@MockK(relaxed = true) private lateinit var accountManager: FxaAccountManager
@MockK(relaxed = true) private lateinit var navController: NavController
@MockK(relaxed = true) private lateinit var loadBookmarkNode: suspend (String) -> BookmarkNode?
@MockK(relaxed = true) private lateinit var showSnackbar: (String) -> Unit
@MockK(relaxed = true) private lateinit var deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit
@MockK(relaxed = true) private lateinit var deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit
@MockK(relaxed = true) private lateinit var invokePendingDeletion: () -> Unit

private lateinit var controller: BookmarkController

private val bookmarkStore = spyk(BookmarkFragmentStore(BookmarkFragmentState(null)))
private val context: Context = mockk(relaxed = true)
private val scope = TestCoroutineScope()
private val clipboardManager: ClipboardManager = mockk(relaxUnitFun = true)
private val navController: NavController = mockk(relaxed = true)
private val sharedViewModel: BookmarksSharedViewModel = mockk()
private val loadBookmarkNode: suspend (String) -> BookmarkNode? = mockk(relaxed = true)
private val showSnackbar: (String) -> Unit = mockk(relaxed = true)
private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true)
private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = mockk(relaxed = true)
private val invokePendingDeletion: () -> Unit = mockk(relaxed = true)

private val homeActivity: HomeActivity = mockk(relaxed = true)
private val services: Services = mockk(relaxed = true)

private val item =
BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0, "Mozilla", "http://mozilla.org", null)
private val subfolder =
Expand Down Expand Up @@ -84,20 +89,15 @@ class BookmarkControllerTest {

@Before
fun setup() {
MockKAnnotations.init(this)
loadBookmarkNode = mockk(relaxed = true)

every { navController.currentDestination } returns mockk {
every { id } returns R.id.bookmarkFragment
every { homeActivity.components.services } returns services
every { navController.currentDestination } returns NavDestination("").apply {
id = R.id.bookmarkFragment
}
every { bookmarkStore.state } returns BookmarkFragmentState(null)
every { bookmarkStore.dispatch(any()) } returns mockk()
every { sharedViewModel.selectedFolder = any() } just runs

controller = DefaultBookmarkController(
activity = homeActivity,
bookmarkStorage = bookmarkStorage,
accountManager = accountManager,
navController = navController,
clipboardManager = clipboardManager,
scope = scope,
Expand Down Expand Up @@ -211,12 +211,12 @@ class BookmarkControllerTest {

@Test
fun `handleBookmarkSelected should show a toast when selecting a root folder`() {
every { homeActivity.resources.getString(R.string.bookmark_cannot_edit_root) } returns "Can't edit default folders"
val errorMessage = context.getString(R.string.bookmark_cannot_edit_root)

controller.handleBookmarkSelected(root)

verify {
showSnackbar("Can't edit default folders")
showSnackbar(errorMessage)
}
}

Expand All @@ -240,24 +240,25 @@ class BookmarkControllerTest {

@Test
fun `handleCopyUrl should copy bookmark url to clipboard and show a toast`() {
every { homeActivity.resources.getString(R.string.url_copied) } returns "URL copied"
val urlCopiedMessage = context.getString(R.string.url_copied)

controller.handleCopyUrl(item)

verifyOrder {
ClipData.newPlainText(item.url, item.url)
showSnackbar("URL copied")
showSnackbar(urlCopiedMessage)
}
}

@Test
fun `handleBookmarkSharing should navigate to the 'Share' fragment`() {
val navDirectionsSlot = slot<NavDirections>()
every { navController.navigate(capture(navDirectionsSlot), null) } just Runs

controller.handleBookmarkSharing(item)

verify {
navController.navigate(withArg<NavDirections> {
assertEquals(R.id.action_global_shareFragment, it.actionId)
}, null)
navController.navigate(navDirectionsSlot.captured, null)
}
}

Expand Down Expand Up @@ -312,7 +313,8 @@ class BookmarkControllerTest {

@Test
fun `handleRequestSync dispatches actions in the correct order`() {
coEvery { bookmarkStorage.getBookmark(any()) } returns tree
every { homeActivity.components.backgroundServices.accountManager } returns mockk(relaxed = true)
coEvery { homeActivity.bookmarkStorage.getBookmark(any()) } returns tree
coEvery { loadBookmarkNode.invoke(any()) } returns tree

controller.handleRequestSync()
Expand Down
10 changes: 4 additions & 6 deletions app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,11 @@ class ShareControllerTest {

@Test
fun `handleShareToApp should start a new sharing activity and close this`() = runBlocking {
val appPackageName = "package"
val appClassName = "activity"
val appShareOption = AppShareOption(
name = "app",
icon = mockk(),
packageName = appPackageName,
activityName = appClassName
packageName = "package",
activityName = "activity"
)
val shareIntent = slot<Intent>()
// Our share Intent uses `FLAG_ACTIVITY_NEW_TASK` but when resolving the startActivity call
Expand All @@ -110,8 +108,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(appPackageName, shareIntent.captured.component!!.packageName)
assertEquals(appClassName, shareIntent.captured.component!!.className)
assertEquals("package", shareIntent.captured.component!!.packageName)
assertEquals("activity", shareIntent.captured.component!!.className)

verify { recentAppStorage.updateRecentApp(appShareOption.activityName) }
verifyOrder {
Expand Down

0 comments on commit 4de4668

Please sign in to comment.