From 1df7a0b27a34e8a58f7b22bb7fd323aad92244e7 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 17 May 2024 18:06:05 +0530 Subject: [PATCH] Fixed the application was crashing when we frequently searching and clicking on any article. * Added test cases for testing this scenario. --- .../kiwixmobile/search/SearchFragmentTest.kt | 17 ++++++ .../kiwix/kiwixmobile/search/SearchRobot.kt | 58 ++++++++++++------- .../kiwixmobile/core/search/SearchFragment.kt | 9 +++ 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt index 5e0519956..2fa2581b6 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt @@ -166,6 +166,23 @@ class SearchFragmentTest : BaseActivityTest() { // go to reader screen pressBack() } + + // Added test for checking the crash scenario where the application was crashing when we + // frequently searched for article, and clicked on the searched item. + search { + // test by searching 10 article and clicking on them + searchAndClickOnArticle(searchQueryForDownloadedZimFile) + searchAndClickOnArticle("A Song") + searchAndClickOnArticle("The Ra") + searchAndClickOnArticle("The Ge") + searchAndClickOnArticle("Wish") + searchAndClickOnArticle("WIFI") + searchAndClickOnArticle("Woman") + searchAndClickOnArticle("Big Ba") + searchAndClickOnArticle("My Wor") + searchAndClickOnArticle("100") + assertArticleLoaded() + } removeTemporaryZimFilesToFreeUpDeviceStorage() LeakAssertions.assertNoLeaks() } diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchRobot.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchRobot.kt index 4b7884fc8..a64cb92de 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchRobot.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchRobot.kt @@ -20,26 +20,27 @@ package org.kiwix.kiwixmobile.search import android.view.KeyEvent import androidx.recyclerview.widget.RecyclerView -import androidx.recyclerview.widget.RecyclerView.ViewHolder import androidx.test.espresso.Espresso.onView import androidx.test.espresso.action.ViewActions.clearText import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.action.ViewActions.typeText import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.contrib.RecyclerViewActions.actionOnItemAtPosition -import androidx.test.espresso.contrib.RecyclerViewActions.scrollToPosition -import androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA -import androidx.test.espresso.matcher.ViewMatchers.isDisplayed +import androidx.test.espresso.matcher.ViewMatchers.hasDescendant import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.espresso.web.sugar.Web.onWebView +import androidx.test.espresso.web.webdriver.DriverAtoms.findElement +import androidx.test.espresso.web.webdriver.Locator import androidx.test.uiautomator.UiDevice import applyWithViewHierarchyPrinting import com.adevinta.android.barista.interaction.BaristaSleepInteractions -import org.hamcrest.Matchers.allOf +import com.adevinta.android.barista.internal.matcher.HelperMatchers.atPosition import org.kiwix.kiwixmobile.BaseRobot import org.kiwix.kiwixmobile.Findable.ViewId import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.testutils.TestUtils +import org.kiwix.kiwixmobile.testutils.TestUtils.testFlakyView fun search(func: SearchRobot.() -> Unit) = SearchRobot().applyWithViewHierarchyPrinting(func) @@ -67,29 +68,24 @@ class SearchRobot : BaseRobot() { } fun searchWithFrequentlyTypedWords(query: String, wait: Long = 0L) { - val searchView = onView(withId(R.id.search_src_text)) - for (char in query) { - searchView.perform(typeText(char.toString())) - if (wait != 0L) { - BaristaSleepInteractions.sleep(wait) + testFlakyView({ + val searchView = onView(withId(R.id.search_src_text)) + for (char in query) { + searchView.perform(typeText(char.toString())) + if (wait != 0L) { + BaristaSleepInteractions.sleep(wait) + } } - } + }) } fun assertSearchSuccessful(searchResult: String) { BaristaSleepInteractions.sleep(TestUtils.TEST_PAUSE_MS_FOR_SEARCH_TEST.toLong()) val recyclerViewId = R.id.search_list - // Scroll to the first position in the RecyclerView - onView(withId(recyclerViewId)).perform(scrollToPosition(0)) - - // Match the view at the first position in the RecyclerView - onView(withText(searchResult)).check( + onView(withId(recyclerViewId)).check( matches( - allOf( - isDisplayed(), - isDescendantOfA(withId(recyclerViewId)) - ) + atPosition(0, hasDescendant(withText(searchResult))) ) ) } @@ -106,4 +102,26 @@ class SearchRobot : BaseRobot() { val searchView = onView(withId(R.id.search_src_text)) searchView.perform(clearText()) } + + private fun openSearchScreen() { + testFlakyView({ onView(withId(R.id.menu_search)).perform(click()) }) + } + + fun searchAndClickOnArticle(searchString: String) { + openSearchScreen() + searchWithFrequentlyTypedWords(searchString) + clickOnSearchItemInSearchList() + } + + fun assertArticleLoaded() { + testFlakyView({ + onWebView() + .withElement( + findElement( + Locator.XPATH, + "//*[contains(text(), 'Big Baby DRAM')]" + ) + ) + }) + } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt index 827eb6cd8..1fb039c31 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt @@ -285,6 +285,15 @@ class SearchFragment : BaseFragment() { } private suspend fun render(state: SearchState) { + // Check if the fragment is visible on the screen. This method called multiple times + // (7-14 times) when an item in the search list is clicked, which leads to unnecessary + // data loading and also causes a crash. + // The issue arises because the searchViewModel takes a moment to detach from the window, + // and during this time, this method is called multiple times due to the rendering process. + // To avoid unnecessary data loading and prevent crashes, we check if the search screen is + // visible to the user before proceeding. If the screen is not visible, + // we skip the data loading process. + // if (!isVisible) return searchMutex.withLock { // `cancelAndJoin` cancels the previous running job and waits for it to completely cancel. renderingJob?.cancelAndJoin()