Merge branch 'tests_reliability' into 'master'

Improve tests reliability, fix feed viewmodel

See merge request pixeldroid/PixelDroid!332
This commit is contained in:
Matthieu 2021-04-30 20:18:55 +00:00
commit d0e03660c5
18 changed files with 140 additions and 133 deletions

View File

@ -103,7 +103,7 @@ dependencies {
implementation "androidx.swiperefreshlayout:swiperefreshlayout:1.1.0"
implementation 'androidx.navigation:navigation-fragment-ktx:2.3.5'
implementation 'androidx.navigation:navigation-ui-ktx:2.3.5'
implementation 'androidx.paging:paging-runtime-ktx:3.0.0-beta03'
implementation 'androidx.paging:paging-runtime-ktx:3.0.0-rc01'
implementation 'androidx.lifecycle:lifecycle-livedata-ktx:2.3.1'
implementation 'androidx.lifecycle:lifecycle-viewmodel-savedstate:2.3.1'
implementation "androidx.lifecycle:lifecycle-runtime-ktx:2.3.1"
@ -113,16 +113,16 @@ dependencies {
implementation "androidx.activity:activity-ktx:1.2.2"
// Use the most recent version of CameraX
def cameraX_version = '1.0.0-rc04'
def cameraX_version = '1.0.0-rc05'
implementation "androidx.camera:camera-core:${cameraX_version}"
implementation "androidx.camera:camera-camera2:${cameraX_version}"
// CameraX Lifecycle library
implementation "androidx.camera:camera-lifecycle:$cameraX_version"
// CameraX View class
implementation 'androidx.camera:camera-view:1.0.0-alpha23'
implementation 'androidx.camera:camera-view:1.0.0-alpha24'
def room_version = "2.3.0-rc01"
def room_version = "2.3.0"
implementation "androidx.room:room-runtime:$room_version"
kapt "androidx.room:room-compiler:$room_version"
implementation "androidx.room:room-ktx:$room_version"
@ -190,7 +190,7 @@ dependencies {
// debugImplementation required vs testImplementation: https://issuetracker.google.com/issues/128612536
//noinspection FragmentGradleConfiguration
stagingImplementation("androidx.fragment:fragment-testing:1.3.2") {
stagingImplementation("androidx.fragment:fragment-testing:1.3.3") {
exclude group:'androidx.test', module:'monitor'
}

View File

@ -19,9 +19,11 @@ import org.junit.After
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TestRule
import org.junit.rules.Timeout
import org.junit.runner.Description
import org.junit.runner.RunWith
import org.junit.runners.model.Statement
@RunWith(AndroidJUnit4::class)
class HomeFeedTest {
@ -30,6 +32,9 @@ class HomeFeedTest {
private lateinit var db: AppDatabase
private lateinit var context: Context
@Rule @JvmField
var repeatRule: RepeatRule = RepeatRule()
@get:Rule
var globalTimeout: Timeout = Timeout.seconds(100)
@ -46,6 +51,10 @@ class HomeFeedTest {
)
db.close()
activityScenario = ActivityScenario.launch(MainActivity::class.java)
waitForView(R.id.username)
onView(withId(R.id.list)).perform(scrollToPosition<StatusViewHolder>(0))
}
@After
fun after() {
@ -53,6 +62,7 @@ class HomeFeedTest {
}
@Test
@RepeatTest
fun clickingTabOnAlbumShowsNextPhoto() {
//Wait for the feed to load
waitForView(R.id.postPager)
@ -126,6 +136,7 @@ class HomeFeedTest {
}*/
@Test
@RepeatTest
fun clickingUsernameOpensProfile() {
waitForView(R.id.username)
@ -136,6 +147,7 @@ class HomeFeedTest {
}
@Test
@RepeatTest
fun clickingProfilePicOpensProfile() {
waitForView(R.id.profilePic)
@ -146,6 +158,7 @@ class HomeFeedTest {
}
@Test
@RepeatTest
fun clickingMentionOpensProfile() {
waitForView(R.id.description)
@ -216,6 +229,7 @@ class HomeFeedTest {
.check(matches(hasDescendant(withId(R.id.comment))))
}*/
@RepeatTest
@Test
fun performClickOnSensitiveWarning() {
waitForView(R.id.username)
@ -231,11 +245,10 @@ class HomeFeedTest {
}
@Test
@RepeatTest
fun performClickOnSensitiveWarningTabs() {
waitForView(R.id.username)
onView(withId(R.id.list)).perform(scrollToPosition<StatusViewHolder>(0))
onView(first(withId(R.id.sensitiveWarning))).check(matches(withEffectiveVisibility(Visibility.VISIBLE)))
onView(withId(R.id.list))

View File

@ -21,9 +21,37 @@ import org.hamcrest.CoreMatchers.allOf
import org.hamcrest.Description
import org.hamcrest.Matcher
import org.hamcrest.Matchers
import org.junit.rules.TestRule
import org.junit.runners.model.Statement
import java.util.concurrent.TimeoutException
@Retention(AnnotationRetention.RUNTIME)
@Target(AnnotationTarget.FUNCTION, AnnotationTarget.ANNOTATION_CLASS)
annotation class RepeatTest(val value: Int = 1)
class RepeatRule : TestRule {
private class RepeatStatement(private val statement: Statement, private val repeat: Int) : Statement() {
@Throws(Throwable::class)
override fun evaluate() {
for (i in 0 until repeat) {
statement.evaluate()
}
}
}
override fun apply(statement: Statement, description: org.junit.runner.Description): Statement {
var result = statement
val repeat = description.getAnnotation(RepeatTest::class.java)
if (repeat != null) {
val times = repeat.value
result = RepeatStatement(statement, times)
}
return result
}
}
fun ViewInteraction.isDisplayed(): Boolean {
return try {
check(matches(ViewMatchers.isDisplayed()))

View File

@ -73,21 +73,20 @@ class MainActivity : BaseActivity() {
setupDrawer()
val tabs: List<() -> Fragment> = listOf(
{
val tabs: List<Fragment> = listOf(
PostFeedFragment<HomeStatusDatabaseEntity>()
.apply {
arguments = Bundle().apply { putBoolean("home", true) }
}
},
{ SearchDiscoverFragment() },
{ CameraFragment() },
{ NotificationsFragment() },
{
,
SearchDiscoverFragment() ,
CameraFragment() ,
NotificationsFragment() ,
PostFeedFragment<PublicFeedStatusDatabaseEntity>()
.apply {
arguments = Bundle().apply { putBoolean("home", false) }
}
}
)
setupTabs(tabs)
@ -270,10 +269,10 @@ class MainActivity : BaseActivity() {
}
private fun setupTabs(tab_array: List<() -> Fragment>){
private fun setupTabs(tab_array: List<Fragment>){
binding.viewPager.adapter = object : FragmentStateAdapter(this) {
override fun createFragment(position: Int): Fragment {
return tab_array[position]()
return tab_array[position]
}
override fun getItemCount(): Int {

View File

@ -12,6 +12,7 @@ import androidx.paging.LoadStateAdapter
import androidx.paging.PagingDataAdapter
import androidx.recyclerview.widget.RecyclerView
import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
import com.google.gson.Gson
import org.pixeldroid.app.R
import org.pixeldroid.app.databinding.ErrorLayoutBinding
import org.pixeldroid.app.databinding.LoadStateFooterViewItemBinding
@ -20,6 +21,7 @@ import org.pixeldroid.app.utils.api.objects.FeedContent
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.launch
import retrofit2.HttpException
/**
* Shows or hides the error in the different FeedFragments
@ -56,24 +58,17 @@ internal fun <T: Any> initAdapter(
if(!progressBar.isVisible && swipeRefreshLayout.isRefreshing) {
// Stop loading spinner when loading is done
swipeRefreshLayout.isRefreshing = loadState.refresh is LoadState.Loading
} else {
// ProgressBar should stop showing as soon as the source stops loading ("source"
// meaning the database, so don't wait on the network)
val sourceLoading = loadState.source.refresh is LoadState.Loading
if(!sourceLoading && recyclerView.size > 0){
recyclerView.isVisible = true
progressBar.isVisible = false
} else if(recyclerView.size == 0
&& loadState.append is LoadState.NotLoading
&& loadState.append.endOfPaginationReached){
progressBar.isVisible = false
showError(motionLayout = motionLayout, errorLayout = errorLayout,
errorText = errorLayout.root.context.getString(R.string.empty_feed))
}
}
// ProgressBar should stop showing as soon as the source stops loading ("source"
// meaning the database, so don't wait on the network)
val sourceLoading = loadState.source.refresh is LoadState.Loading
if (!sourceLoading && adapter.itemCount > 0) {
recyclerView.isVisible = true
progressBar.isVisible = false
}
// Toast on any error, regardless of whether it came from RemoteMediator or PagingSource
// Show any error, regardless of whether it came from RemoteMediator or PagingSource
val errorState = loadState.source.append as? LoadState.Error
?: loadState.source.prepend as? LoadState.Error
?: loadState.source.refresh as? LoadState.Error
@ -81,21 +76,37 @@ internal fun <T: Any> initAdapter(
?: loadState.prepend as? LoadState.Error
?: loadState.refresh as? LoadState.Error
errorState?.let {
showError(motionLayout = motionLayout, errorLayout = errorLayout, errorText = it.error.toString())
val error: String = (it.error as? HttpException)?.response()?.errorBody()?.string()?.ifEmpty { null }?.let { s ->
Gson().fromJson(s, org.pixeldroid.app.utils.api.objects.Error::class.java)?.error?.ifBlank { null }
} ?: it.error.localizedMessage.orEmpty()
showError(motionLayout = motionLayout, errorLayout = errorLayout, errorText = error)
}
// If the state is not an error, hide the error layout, or show message that the feed is empty
if(errorState == null) {
showError(motionLayout = motionLayout, errorLayout = errorLayout, show = false, errorText = "")
if (adapter.itemCount == 0
&& loadState.append is LoadState.NotLoading
&& loadState.append.endOfPaginationReached
) {
progressBar.isVisible = false
showError(
motionLayout = motionLayout, errorLayout = errorLayout,
errorText = errorLayout.root.context.getString(R.string.empty_feed)
)
} else {
showError(motionLayout = motionLayout, errorLayout = errorLayout, show = false, errorText = "")
}
}
}
}
fun launch(
job: Job?, lifecycleScope: LifecycleCoroutineScope, viewModel: FeedViewModel<FeedContent>,
pagingDataAdapter: PagingDataAdapter<FeedContent, RecyclerView.ViewHolder>): Job {
fun <T: FeedContent> launch(
job: Job?, lifecycleScope: LifecycleCoroutineScope, viewModel: FeedViewModel<T>,
pagingDataAdapter: PagingDataAdapter<T, RecyclerView.ViewHolder>): Job {
// Make sure we cancel the previous job before creating a new one
job?.cancel()
return lifecycleScope.launch {
viewModel.flow().collectLatest {
viewModel.flow.collectLatest {
pagingDataAdapter.submitData(it)
}
}

View File

@ -8,21 +8,16 @@ import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.lifecycleScope
import androidx.paging.*
import androidx.paging.LoadState.*
import androidx.recyclerview.widget.RecyclerView
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.distinctUntilChangedBy
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.launch
import kotlinx.coroutines.flow.*
import org.pixeldroid.app.databinding.FragmentFeedBinding
import org.pixeldroid.app.posts.feeds.initAdapter
import org.pixeldroid.app.utils.BaseFragment
import org.pixeldroid.app.utils.api.objects.FeedContentDatabase
import org.pixeldroid.app.utils.db.AppDatabase
import org.pixeldroid.app.utils.db.dao.feedContent.FeedContentDao
import org.pixeldroid.app.utils.BaseFragment
import org.pixeldroid.app.posts.feeds.initAdapter
import org.pixeldroid.app.utils.api.objects.FeedContentDatabase
/**
* A fragment representing a list of [FeedContentDatabase] items that are cached by the database.
@ -43,7 +38,7 @@ open class CachedFeedFragment<T: FeedContentDatabase> : BaseFragment() {
// Make sure we cancel the previous job before creating a new one
job?.cancel()
job = lifecycleScope.launchWhenStarted {
viewModel.flow().collectLatest {
viewModel.flow.collectLatest {
adapter.submitData(it)
}
}
@ -51,12 +46,14 @@ open class CachedFeedFragment<T: FeedContentDatabase> : BaseFragment() {
internal fun initSearch() {
// Scroll to top when the list is refreshed from network.
lifecycleScope.launch {
lifecycleScope.launchWhenStarted {
adapter.loadStateFlow
// Only emit when REFRESH LoadState for RemoteMediator changes.
.distinctUntilChangedBy { it.refresh }
.distinctUntilChangedBy {
it.refresh
}
// Only react to cases where Remote REFRESH completes i.e., NotLoading.
.filter { it.refresh is LoadState.NotLoading }
.filter { it.refresh is NotLoading}
.collect { binding.list.scrollToPosition(0) }
}
}
@ -73,12 +70,7 @@ open class CachedFeedFragment<T: FeedContentDatabase> : BaseFragment() {
initAdapter(binding.progressBar, binding.swipeRefreshLayout,
binding.list, binding.motionLayout, binding.errorLayout, adapter)
//binding.progressBar.visibility = View.GONE
binding.swipeRefreshLayout.setOnRefreshListener {
//It shouldn't be necessary to also retry() in addition to refresh(),
//but if we don't do this, reloads after an error fail immediately...
// https://issuetracker.google.com/issues/173438474
adapter.retry()
adapter.refresh()
}

View File

@ -26,19 +26,8 @@ import kotlinx.coroutines.flow.Flow
* ViewModel for the cached feeds.
* The ViewModel works with the [FeedContentRepository] to get the data.
*/
class FeedViewModel<T: FeedContentDatabase>(private val repository: FeedContentRepository<T>) : ViewModel() {
private var currentResult: Flow<PagingData<T>>? = null
class FeedViewModel<T: FeedContentDatabase>(repository: FeedContentRepository<T>) : ViewModel() {
@ExperimentalPagingApi
fun flow(): Flow<PagingData<T>> {
val lastResult = currentResult
if (lastResult != null) {
return lastResult
}
val newResult: Flow<PagingData<T>> = repository.stream()
.cachedIn(viewModelScope)
currentResult = newResult
return newResult
}
val flow: Flow<PagingData<T>> = repository.stream().cachedIn(viewModelScope)
}

View File

@ -53,10 +53,10 @@ class NotificationsFragment : CachedFeedFragment<Notification>() {
// get the view model
@Suppress("UNCHECKED_CAST")
viewModel = ViewModelProvider(
this,
requireActivity(),
ViewModelFactory(db, db.notificationDao(), NotificationsRemoteMediator(apiHolder, db))
)
.get(FeedViewModel::class.java) as FeedViewModel<Notification>
.get("notifications", FeedViewModel::class.java) as FeedViewModel<Notification>
launch()
initSearch()

View File

@ -20,6 +20,7 @@ import org.pixeldroid.app.posts.feeds.cachedFeeds.ViewModelFactory
import org.pixeldroid.app.utils.api.objects.FeedContentDatabase
import org.pixeldroid.app.utils.api.objects.Status
import org.pixeldroid.app.utils.displayDimensionsInPx
import kotlin.properties.Delegates
/**
@ -32,14 +33,17 @@ class PostFeedFragment<T: FeedContentDatabase>: CachedFeedFragment<T>() {
private lateinit var mediator: RemoteMediator<Int, T>
private lateinit var dao: FeedContentDao<T>
private var home by Delegates.notNull<Boolean>()
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
adapter = PostsAdapter(requireContext().displayDimensionsInPx())
home = requireArguments().get("home") as Boolean
@Suppress("UNCHECKED_CAST")
if (requireArguments().get("home") as Boolean){
if (home){
mediator = HomeFeedRemoteMediator(apiHolder, db) as RemoteMediator<Int, T>
dao = db.homePostDao() as FeedContentDao<T>
}
@ -59,8 +63,8 @@ class PostFeedFragment<T: FeedContentDatabase>: CachedFeedFragment<T>() {
// get the view model
@Suppress("UNCHECKED_CAST")
viewModel = ViewModelProvider(this, ViewModelFactory(db, dao, mediator))
.get(FeedViewModel::class.java) as FeedViewModel<T>
viewModel = ViewModelProvider(requireActivity(), ViewModelFactory(db, dao, mediator))
.get(if(home) "home" else "public", FeedViewModel::class.java) as FeedViewModel<T>
launch()
initSearch()
@ -70,12 +74,8 @@ class PostFeedFragment<T: FeedContentDatabase>: CachedFeedFragment<T>() {
inner class PostsAdapter(private val displayDimensionsInPx: Pair<Int, Int>) : PagingDataAdapter<T, RecyclerView.ViewHolder>(
object : DiffUtil.ItemCallback<T>() {
override fun areItemsTheSame(oldItem: T, newItem: T): Boolean {
return oldItem.id == newItem.id
}
override fun areContentsTheSame(oldItem: T, newItem: T): Boolean =
oldItem.id == newItem.id
override fun areItemsTheSame (oldItem: T, newItem: T): Boolean = oldItem.id == newItem.id
override fun areContentsTheSame(oldItem: T, newItem: T): Boolean = oldItem.id == newItem.id
}
) {

View File

@ -10,20 +10,8 @@ import kotlinx.coroutines.flow.Flow
* ViewModel for the uncached feeds.
* The ViewModel works with the different [UncachedContentRepository]s to get the data.
*/
class FeedViewModel<T: FeedContent>(private val repository: UncachedContentRepository<T>) : ViewModel() {
private var currentResult: Flow<PagingData<T>>? = null
fun flow(): Flow<PagingData<T>> {
val lastResult = currentResult
if (lastResult != null) {
return lastResult
}
val newResult: Flow<PagingData<T>> = repository.getStream()
.cachedIn(viewModelScope)
currentResult = newResult
return newResult
}
class FeedViewModel<T: FeedContent>(repository: UncachedContentRepository<T>) : ViewModel() {
val flow: Flow<PagingData<T>> = repository.getStream().cachedIn(viewModelScope)
}
/**

View File

@ -37,10 +37,7 @@ open class UncachedFeedFragment<T: FeedContent> : BaseFragment() {
internal fun launch() {
@Suppress("UNCHECKED_CAST")
job = launch(job, lifecycleScope,
viewModel as FeedViewModel<FeedContent>,
adapter as PagingDataAdapter<FeedContent, RecyclerView.ViewHolder>)
job = launch(job, lifecycleScope, viewModel, adapter)
}
internal fun initSearch() {
@ -68,9 +65,6 @@ open class UncachedFeedFragment<T: FeedContent> : BaseFragment() {
binding.motionLayout, binding.errorLayout, adapter)
binding.swipeRefreshLayout.setOnRefreshListener {
//It shouldn't be necessary to also retry() in addition to refresh(),
//but if we don't do this, reloads after an error fail immediately...
adapter.retry()
adapter.refresh()
}

View File

@ -52,7 +52,7 @@ class AccountListFragment : UncachedFeedFragment<Account>() {
// get the view model
@Suppress("UNCHECKED_CAST")
viewModel = ViewModelProvider(this, ViewModelFactory(
viewModel = ViewModelProvider(requireActivity(), ViewModelFactory(
FollowersContentRepository(
apiHolder.setToCurrentUser(),
id,
@ -60,7 +60,7 @@ class AccountListFragment : UncachedFeedFragment<Account>() {
)
)
)
.get(FeedViewModel::class.java) as FeedViewModel<Account>
.get("accountList", FeedViewModel::class.java) as FeedViewModel<Account>
launch()
initSearch()

View File

@ -41,25 +41,22 @@ class FollowersPagingSource(
throw HttpException(response)
}
val nextPosition: String = if(response.headers()["Link"] != null){
//Header is of the form:
val nextPosition: String = response.headers()["Link"]
// Header is of the form:
// Link: <https://mastodon.social/api/v1/accounts/1/followers?limit=2&max_id=7628164>; rel="next", <https://mastodon.social/api/v1/accounts/1/followers?limit=2&since_id=7628165>; rel="prev"
// So we want the first max_id value. In case there are arguments after
// the max_id in the URL, we make sure to stop at the first '?'
response.headers()["Link"]
.orEmpty()
.substringAfter("max_id=", "")
.substringBefore('?', "")
.substringBefore('>', "")
} else {
// No Link header, so we just increment the position value
?.substringAfter("max_id=", "")
?.substringBefore('?', "")
?.substringBefore('>', "")
?: // No Link header, so we just increment the position value (Pixelfed case)
(position?.toBigIntegerOrNull() ?: 1.toBigInteger()).inc().toString()
}
LoadResult.Page(
data = accounts,
prevKey = null,
nextKey = if (accounts.isEmpty()) null else nextPosition
nextKey = if (accounts.isEmpty() or nextPosition.isEmpty()) null else nextPosition
)
} catch (exception: IOException) {
LoadResult.Error(exception)
@ -68,8 +65,5 @@ class FollowersPagingSource(
}
}
override fun getRefreshKey(state: PagingState<String, Account>): String? =
state.anchorPosition?.run {
state.closestItemToPosition(this)?.id
}
override fun getRefreshKey(state: PagingState<String, Account>): String? = null
}

View File

@ -37,14 +37,14 @@ class SearchAccountFragment : UncachedFeedFragment<Account>() {
// get the view model
@Suppress("UNCHECKED_CAST")
viewModel = ViewModelProvider(this, ViewModelFactory(
viewModel = ViewModelProvider(requireActivity(), ViewModelFactory(
SearchContentRepository<Account>(
apiHolder.setToCurrentUser(),
Results.SearchType.accounts,
query
)
)
).get(FeedViewModel::class.java) as FeedViewModel<Account>
).get("searchAccounts", FeedViewModel::class.java) as FeedViewModel<Account>
launch()
initSearch()

View File

@ -44,7 +44,7 @@ class SearchHashtagFragment : UncachedFeedFragment<Tag>() {
// get the view model
@Suppress("UNCHECKED_CAST")
viewModel = ViewModelProvider(this, ViewModelFactory(
viewModel = ViewModelProvider(requireActivity(), ViewModelFactory(
SearchContentRepository<Tag>(
apiHolder.setToCurrentUser(),
Results.SearchType.hashtags,
@ -52,7 +52,7 @@ class SearchHashtagFragment : UncachedFeedFragment<Tag>() {
)
)
)
.get(FeedViewModel::class.java) as FeedViewModel<Tag>
.get("searchHashtag", FeedViewModel::class.java) as FeedViewModel<Tag>
launch()
initSearch()

View File

@ -46,8 +46,9 @@ class SearchPagingSource<T: FeedContent>(
}
}
override fun getRefreshKey(state: PagingState<Int, T>): Int? =
state.anchorPosition?.run {
state.closestItemToPosition(this)?.id?.toIntOrNull()
}
/**
* FIXME if implemented with [PagingState.anchorPosition], this breaks refreshes? How is this
* supposed to work?
*/
override fun getRefreshKey(state: PagingState<Int, T>): Int? = null
}

View File

@ -41,7 +41,7 @@ class SearchPostsFragment : UncachedFeedFragment<Status>() {
// get the view model
@Suppress("UNCHECKED_CAST")
viewModel = ViewModelProvider(this, ViewModelFactory(
viewModel = ViewModelProvider(requireActivity(), ViewModelFactory(
SearchContentRepository<Status>(
apiHolder.setToCurrentUser(),
Results.SearchType.statuses,
@ -49,7 +49,7 @@ class SearchPostsFragment : UncachedFeedFragment<Status>() {
)
)
)
.get(FeedViewModel::class.java) as FeedViewModel<Status>
.get("searchPosts", FeedViewModel::class.java) as FeedViewModel<Status>
launch()
initSearch()

View File

@ -91,9 +91,7 @@ class ProfileActivity : BaseActivity() {
}
setContent(account)
@Suppress("UNCHECKED_CAST")
job = launch(job, lifecycleScope, viewModel as FeedViewModel<FeedContent>,
profileAdapter as PagingDataAdapter<FeedContent, RecyclerView.ViewHolder>)
job = launch(job, lifecycleScope, viewModel, profileAdapter)
}
/**