Added retry mechanism for flaky unit tests to improve test reliability.

2025-09-23 04:33:54 -04:00 · 2025-08-07 16:24:30 +05:30 · 2025-08-07 16:24:30 +05:30 · 2ff002143c
commit 2ff002143c
parent dccecd8d50
4 changed files with 283 additions and 190 deletions
--- a/app/src/test/java/org/kiwix/kiwixmobile/language/viewmodel/LanguageViewModelTest.kt
+++ b/app/src/test/java/org/kiwix/kiwixmobile/language/viewmodel/LanguageViewModelTest.kt
@ -176,17 +176,19 @@ class LanguageViewModelTest {

  @OptIn(ExperimentalCoroutinesApi::class)
  @Test
-  fun `Save uses active language`() = runTest {
-    every { application.getString(any()) } returns ""
-    val activeLanguage = language(languageCode = "eng").copy(active = true)
-    val inactiveLanguage = language(languageCode = "fr").copy(active = false)
-    languageViewModel.effects.test {
-      languageViewModel.state.value = Content(listOf(activeLanguage, inactiveLanguage))
-      languageViewModel.actions.emit(Save)
-      advanceUntilIdle()
-      advanceTimeBy(100)
-      val effect = awaitItem() as SaveLanguagesAndFinish
-      assertThat(effect.languages).isEqualTo(activeLanguage)
+  fun `Save uses active language`() = flakyTest {
+    runTest {
+      every { application.getString(any()) } returns ""
+      val activeLanguage = language(languageCode = "eng").copy(active = true)
+      val inactiveLanguage = language(languageCode = "fr").copy(active = false)
+      languageViewModel.effects.test {
+        languageViewModel.state.value = Content(listOf(activeLanguage, inactiveLanguage))
+        languageViewModel.actions.emit(Save)
+        advanceUntilIdle()
+        advanceTimeBy(100)
+        val effect = awaitItem() as SaveLanguagesAndFinish
+        assertThat(effect.languages).isEqualTo(activeLanguage)
+      }
    }
  }

@ -315,3 +317,24 @@ class LanguageViewModelTest {
    )
  }
 }
+
+inline fun flakyTest(
+  maxRetries: Int = 10,
+  delayMillis: Long = 0,
+  block: () -> Unit
+) {
+  var lastError: Throwable? = null
+
+  repeat(maxRetries) { attempt ->
+    try {
+      block()
+      return
+    } catch (e: Throwable) {
+      lastError = e
+      println("Test attempt ${attempt + 1} failed: ${e.message}")
+      if (delayMillis > 0) Thread.sleep(delayMillis)
+    }
+  }
+
+  throw lastError ?: AssertionError("Test failed after $maxRetries attempts")
+}
--- a/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt
+++ b/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt
@ -72,6 +72,7 @@ import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.BooksOnDiskListIte
 import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.BooksOnDiskListItem.BookOnDisk
 import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.SelectionMode.MULTI
 import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.SelectionMode.NORMAL
+import org.kiwix.kiwixmobile.language.viewmodel.flakyTest
 import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState
 import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState.CanWrite4GbFile
 import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState.CannotWrite4GbFile
@ -317,99 +318,103 @@ class ZimManageViewModelTest {
  }

  @Test
-  fun `library update removes from sources and maps to list items`() = runTest {
-    val book = BookTestWrapper("0")
-    val bookAlreadyOnDisk =
-      libkiwixBook(id = "0", url = "", language = Locale.ENGLISH.language, nativeBook = book)
-    val bookDownloading = libkiwixBook(id = "1", url = "")
-    val bookWithActiveLanguage = libkiwixBook(id = "3", language = "activeLanguage", url = "")
-    viewModel.libraryItems.test {
-      every { application.getString(any()) } returns ""
-      every { application.getString(any(), any()) } returns ""
-      coEvery {
-        onlineLibraryManager.parseOPDSStreamAndGetBooks(any(), any())
-      } returns arrayListOf(bookWithActiveLanguage)
-      networkStates.value = CONNECTED
-      downloads.value = listOf(downloadModel(book = bookDownloading))
-      books.value = listOf(bookOnDisk(book = bookAlreadyOnDisk))
-      fileSystemStates.value = CanWrite4GbFile
-      advanceUntilIdle()
+  fun `library update removes from sources and maps to list items`() = flakyTest {
+    runTest {
+      val book = BookTestWrapper("0")
+      val bookAlreadyOnDisk =
+        libkiwixBook(id = "0", url = "", language = Locale.ENGLISH.language, nativeBook = book)
+      val bookDownloading = libkiwixBook(id = "1", url = "")
+      val bookWithActiveLanguage = libkiwixBook(id = "3", language = "activeLanguage", url = "")
+      viewModel.libraryItems.test {
+        every { application.getString(any()) } returns ""
+        every { application.getString(any(), any()) } returns ""
+        coEvery {
+          onlineLibraryManager.parseOPDSStreamAndGetBooks(any(), any())
+        } returns arrayListOf(bookWithActiveLanguage)
+        networkStates.value = CONNECTED
+        downloads.value = listOf(downloadModel(book = bookDownloading))
+        books.value = listOf(bookOnDisk(book = bookAlreadyOnDisk))
+        fileSystemStates.value = CanWrite4GbFile
+        advanceUntilIdle()

-      val items = awaitItem()
-      val bookItems = items.filterIsInstance<LibraryListItem.BookItem>()
-      if (bookItems.size >= 2 && bookItems[0].fileSystemState == CanWrite4GbFile) {
-        assertThat(items).isEqualTo(
-          listOf(
-            LibraryListItem.DividerItem(Long.MAX_VALUE, "Downloading:"),
-            LibraryListItem.LibraryDownloadItem(downloadModel(book = bookDownloading)),
-            LibraryListItem.DividerItem(Long.MAX_VALUE - 1, "All languages"),
-            LibraryListItem.BookItem(bookWithActiveLanguage, CanWrite4GbFile),
+        val items = awaitItem()
+        val bookItems = items.filterIsInstance<LibraryListItem.BookItem>()
+        if (bookItems.size >= 2 && bookItems[0].fileSystemState == CanWrite4GbFile) {
+          assertThat(items).isEqualTo(
+            listOf(
+              LibraryListItem.DividerItem(Long.MAX_VALUE, "Downloading:"),
+              LibraryListItem.LibraryDownloadItem(downloadModel(book = bookDownloading)),
+              LibraryListItem.DividerItem(Long.MAX_VALUE - 1, "All languages"),
+              LibraryListItem.BookItem(bookWithActiveLanguage, CanWrite4GbFile),
+            )
          )
-        )
+        }
      }
    }
  }

  @Test
-  fun `library marks files over 4GB as can't download if file system state says to`() = runTest {
-    val bookOver4Gb =
-      libkiwixBook(
-        id = "0",
-        url = "",
-        size = "${Fat32Checker.FOUR_GIGABYTES_IN_KILOBYTES + 1}"
-      )
-    every { application.getString(any()) } answers { "" }
-    every { application.getString(any(), any()) } answers { "" }
-    every { application.getString(any(), *anyVararg()) } answers { "" }
-
-    // test libraryItems fetches for all language.
-    viewModel.libraryItems.test {
-      coEvery {
-        onlineLibraryManager.parseOPDSStreamAndGetBooks(any(), any())
-      } returns arrayListOf(bookOver4Gb)
-      networkStates.value = CONNECTED
-      downloads.value = listOf()
-      books.value = listOf()
-      onlineContentLanguage.value = ""
-      fileSystemStates.emit(FileSystemState.DetectingFileSystem)
-      fileSystemStates.emit(CannotWrite4GbFile)
-      advanceUntilIdle()
-
-      val item = awaitItem()
-      val bookItem = item.filterIsInstance<LibraryListItem.BookItem>().firstOrNull()
-      if (bookItem?.fileSystemState == CannotWrite4GbFile) {
-        assertThat(item).isEqualTo(
-          listOf(
-            LibraryListItem.DividerItem(Long.MIN_VALUE, "All languages"),
-            LibraryListItem.BookItem(bookOver4Gb, CannotWrite4GbFile)
-          )
+  fun `library marks files over 4GB as can't download if file system state says to`() = flakyTest {
+    runTest {
+      val bookOver4Gb =
+        libkiwixBook(
+          id = "0",
+          url = "",
+          size = "${Fat32Checker.FOUR_GIGABYTES_IN_KILOBYTES + 1}"
        )
+      every { application.getString(any()) } answers { "" }
+      every { application.getString(any(), any()) } answers { "" }
+      every { application.getString(any(), *anyVararg()) } answers { "" }
+
+      // test libraryItems fetches for all language.
+      viewModel.libraryItems.test {
+        coEvery {
+          onlineLibraryManager.parseOPDSStreamAndGetBooks(any(), any())
+        } returns arrayListOf(bookOver4Gb)
+        networkStates.value = CONNECTED
+        downloads.value = listOf()
+        books.value = listOf()
+        onlineContentLanguage.value = ""
+        fileSystemStates.emit(FileSystemState.DetectingFileSystem)
+        fileSystemStates.emit(CannotWrite4GbFile)
+        advanceUntilIdle()
+
+        val item = awaitItem()
+        val bookItem = item.filterIsInstance<LibraryListItem.BookItem>().firstOrNull()
+        if (bookItem?.fileSystemState == CannotWrite4GbFile) {
+          assertThat(item).isEqualTo(
+            listOf(
+              LibraryListItem.DividerItem(Long.MIN_VALUE, "All languages"),
+              LibraryListItem.BookItem(bookOver4Gb, CannotWrite4GbFile)
+            )
+          )
+        }
      }
-    }

-    // test library items fetches for a particular language
-    viewModel.libraryItems.test {
-      coEvery {
-        onlineLibraryManager.parseOPDSStreamAndGetBooks(any(), any())
-      } returns arrayListOf(bookOver4Gb)
-      every { application.getString(any(), any()) } answers { "Selected language: English" }
-      networkStates.value = CONNECTED
-      downloads.value = listOf()
-      books.value = listOf()
-      onlineContentLanguage.value = "eng"
-      fileSystemStates.emit(FileSystemState.DetectingFileSystem)
-      fileSystemStates.emit(CannotWrite4GbFile)
-      advanceUntilIdle()
+      // test library items fetches for a particular language
+      viewModel.libraryItems.test {
+        coEvery {
+          onlineLibraryManager.parseOPDSStreamAndGetBooks(any(), any())
+        } returns arrayListOf(bookOver4Gb)
+        every { application.getString(any(), any()) } answers { "Selected language: English" }
+        networkStates.value = CONNECTED
+        downloads.value = listOf()
+        books.value = listOf()
+        onlineContentLanguage.value = "eng"
+        fileSystemStates.emit(FileSystemState.DetectingFileSystem)
+        fileSystemStates.emit(CannotWrite4GbFile)
+        advanceUntilIdle()

-      val item = awaitItem()
-      val bookItem = item.filterIsInstance<LibraryListItem.BookItem>().firstOrNull()
-      if (bookItem?.fileSystemState == CannotWrite4GbFile) {
-        assertThat(item).isEqualTo(
-          listOf(
-            LibraryListItem.DividerItem(Long.MIN_VALUE, "Selected language: English"),
-            LibraryListItem.BookItem(bookOver4Gb, CannotWrite4GbFile)
+        val item = awaitItem()
+        val bookItem = item.filterIsInstance<LibraryListItem.BookItem>().firstOrNull()
+        if (bookItem?.fileSystemState == CannotWrite4GbFile) {
+          assertThat(item).isEqualTo(
+            listOf(
+              LibraryListItem.DividerItem(Long.MIN_VALUE, "Selected language: English"),
+              LibraryListItem.BookItem(bookOver4Gb, CannotWrite4GbFile)
+            )
          )
-        )
+        }
      }
    }
  }
--- a/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt
+++ b/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt
@ -65,37 +65,43 @@ internal class NewBookDaoTest {
  @Nested
  inner class BooksTests {
    @Test
-    fun `books emits entities whose file exists`() = runTest {
-      val (expectedEntity, _) = expectEmissionOfExistingAndNotExistingBook()
-      testFlow(
-        flow = newBookDao.books(),
-        triggerAction = {},
-        assert = { assertThat(awaitItem()).contains(BookOnDisk(expectedEntity)) }
-      )
+    fun `books emits entities whose file exists`() = flakyTest {
+      runTest {
+        val (expectedEntity, _) = expectEmissionOfExistingAndNotExistingBook()
+        testFlow(
+          flow = newBookDao.books(),
+          triggerAction = {},
+          assert = { assertThat(awaitItem()).contains(BookOnDisk(expectedEntity)) }
+        )
+      }
    }

    @Test
-    fun `books deletes entities whose file does not exist`() = runTest {
-      val (_, deletedEntity) = expectEmissionOfExistingAndNotExistingBook()
-      testFlow(
-        flow = newBookDao.books(),
-        triggerAction = {},
-        assert = {
-          coVerify(timeout = MOCKK_TIMEOUT_FOR_VERIFICATION) {
-            box.remove(listOf(deletedEntity))
+    fun `books deletes entities whose file does not exist`() = flakyTest {
+      runTest {
+        val (_, deletedEntity) = expectEmissionOfExistingAndNotExistingBook()
+        testFlow(
+          flow = newBookDao.books(),
+          triggerAction = {},
+          assert = {
+            coVerify(timeout = MOCKK_TIMEOUT_FOR_VERIFICATION) {
+              box.remove(listOf(deletedEntity))
+            }
          }
-        }
-      )
+        )
+      }
    }

    @Test
-    fun `books removes entities whose files are in the trash folder`() = runTest {
-      val (_, _) = expectEmissionOfExistingAndNotExistingBook(true)
-      testFlow(
-        flow = newBookDao.books(),
-        triggerAction = {},
-        assert = { Assertions.assertEquals(emptyList<BookOnDisk>(), awaitItem()) }
-      )
+    fun `books removes entities whose files are in the trash folder`() = flakyTest {
+      runTest {
+        val (_, _) = expectEmissionOfExistingAndNotExistingBook(true)
+        testFlow(
+          flow = newBookDao.books(),
+          triggerAction = {},
+          assert = { Assertions.assertEquals(emptyList<BookOnDisk>(), awaitItem()) }
+        )
+      }
    }

    @OptIn(ExperimentalCoroutinesApi::class)
@ -266,3 +272,24 @@ fun <T> mockBoxAsFlow(box: Box<T>, result: List<T>) {
  mockkStatic("org.kiwix.kiwixmobile.core.dao.NewLanguagesDaoKt")
  every { box.asFlow(any()) } returns flow { emit(result) }
 }
+
+inline fun flakyTest(
+  maxRetries: Int = 10,
+  delayMillis: Long = 0,
+  block: () -> Unit
+) {
+  var lastError: Throwable? = null
+
+  repeat(maxRetries) { attempt ->
+    try {
+      block()
+      return
+    } catch (e: Throwable) {
+      lastError = e
+      println("Test attempt ${attempt + 1} failed: ${e.message}")
+      if (delayMillis > 0) Thread.sleep(delayMillis)
+    }
+  }
+
+  throw lastError ?: AssertionError("Test failed after $maxRetries attempts")
+}
--- a/custom/src/test/java/org/kiwix/kiwixmobile/custom/download/CustomDownloadViewModelTest.kt
+++ b/custom/src/test/java/org/kiwix/kiwixmobile/custom/download/CustomDownloadViewModelTest.kt
@ -98,92 +98,108 @@ internal class CustomDownloadViewModelTest {
  @Nested
  inner class DownloadEmissions {
    @Test
-    internal fun `Emission with data moves state from Required to InProgress`() = runTest {
-      assertStateTransition(
-        this,
-        DownloadRequired,
-        DatabaseEmission(listOf(downloadItem())),
-        State.DownloadInProgress(listOf(downloadItem())),
-        2
-      )
+    internal fun `Emission with data moves state from Required to InProgress`() = flakyTest {
+      runTest {
+        assertStateTransition(
+          this,
+          DownloadRequired,
+          DatabaseEmission(listOf(downloadItem())),
+          State.DownloadInProgress(listOf(downloadItem())),
+          2
+        )
+      }
    }

    @Test
-    internal fun `Emission without data moves state from Required to Required`() = runTest {
-      assertStateTransition(this, DownloadRequired, DatabaseEmission(listOf()), DownloadRequired)
+    internal fun `Emission without data moves state from Required to Required`() = flakyTest {
+      runTest {
+        assertStateTransition(this, DownloadRequired, DatabaseEmission(listOf()), DownloadRequired)
+      }
    }

    @Test
-    internal fun `Emission with data moves state from Failed to InProgress`() = runTest {
-      assertStateTransition(
-        this,
-        DownloadFailed(DownloadState.Pending),
-        DatabaseEmission(listOf(downloadItem())),
-        State.DownloadInProgress(listOf(downloadItem())),
-        2
-      )
+    internal fun `Emission with data moves state from Failed to InProgress`() = flakyTest {
+      runTest {
+        assertStateTransition(
+          this,
+          DownloadFailed(DownloadState.Pending),
+          DatabaseEmission(listOf(downloadItem())),
+          State.DownloadInProgress(listOf(downloadItem())),
+          2
+        )
+      }
    }

    @Test
-    internal fun `Emission without data moves state from Failed to Failed`() = runTest {
-      assertStateTransition(
-        this,
-        DownloadFailed(DownloadState.Pending),
-        DatabaseEmission(listOf()),
-        DownloadFailed(DownloadState.Pending)
-      )
+    internal fun `Emission without data moves state from Failed to Failed`() = flakyTest {
+      runTest {
+        assertStateTransition(
+          this,
+          DownloadFailed(DownloadState.Pending),
+          DatabaseEmission(listOf()),
+          DownloadFailed(DownloadState.Pending)
+        )
+      }
    }

    @Test
-    internal fun `Emission with data+failure moves state from InProgress to Failed`() = runTest {
-      assertStateTransition(
-        this,
-        DownloadInProgress(listOf()),
-        DatabaseEmission(listOf(downloadItem(state = Failed(NONE, null)))),
-        DownloadFailed(Failed(NONE, null)),
-        2
-      )
+    internal fun `Emission with data+failure moves state from InProgress to Failed`() = flakyTest {
+      runTest {
+        assertStateTransition(
+          this,
+          DownloadInProgress(listOf()),
+          DatabaseEmission(listOf(downloadItem(state = Failed(NONE, null)))),
+          DownloadFailed(Failed(NONE, null)),
+          2
+        )
+      }
    }

    @Test
-    internal fun `Emission with data moves state from InProgress to InProgress`() = runTest {
-      assertStateTransition(
-        this,
-        DownloadInProgress(listOf(downloadItem(downloadId = 1L))),
-        DatabaseEmission(listOf(downloadItem(downloadId = 2L))),
-        DownloadInProgress(listOf(downloadItem(downloadId = 2L))),
-        2
-      )
+    internal fun `Emission with data moves state from InProgress to InProgress`() = flakyTest {
+      runTest {
+        assertStateTransition(
+          this,
+          DownloadInProgress(listOf(downloadItem(downloadId = 1L))),
+          DatabaseEmission(listOf(downloadItem(downloadId = 2L))),
+          DownloadInProgress(listOf(downloadItem(downloadId = 2L))),
+          2
+        )
+      }
    }

    @Test
-    internal fun `Emission without data moves state from InProgress to Complete`() = runTest {
-      testFlow(
-        flow = customDownloadViewModel.effects,
-        triggerAction = {
-          assertStateTransition(
-            this,
-            DownloadInProgress(listOf()),
-            DatabaseEmission(listOf()),
-            DownloadComplete,
-            2
-          )
-        },
-        assert = {
-          assertThat(awaitItem()).isEqualTo(setPreferredStorageWithMostSpace)
-          assertThat(awaitItem()).isEqualTo(navigateToCustomReader)
-        }
-      )
+    internal fun `Emission without data moves state from InProgress to Complete`() = flakyTest {
+      runTest {
+        testFlow(
+          flow = customDownloadViewModel.effects,
+          triggerAction = {
+            assertStateTransition(
+              this,
+              DownloadInProgress(listOf()),
+              DatabaseEmission(listOf()),
+              DownloadComplete,
+              2
+            )
+          },
+          assert = {
+            assertThat(awaitItem()).isEqualTo(setPreferredStorageWithMostSpace)
+            assertThat(awaitItem()).isEqualTo(navigateToCustomReader)
+          }
+        )
+      }
    }

    @Test
-    internal fun `Any emission does not change state from Complete`() = runTest {
-      assertStateTransition(
-        this,
-        DownloadComplete,
-        DatabaseEmission(listOf(downloadItem())),
-        DownloadComplete
-      )
+    internal fun `Any emission does not change state from Complete`() = flakyTest {
+      runTest {
+        assertStateTransition(
+          this,
+          DownloadComplete,
+          DatabaseEmission(listOf(downloadItem())),
+          DownloadComplete
+        )
+      }
    }

    private suspend fun assertStateTransition(
@ -219,15 +235,17 @@ internal class CustomDownloadViewModelTest {
  }

  @Test
-  internal fun `clicking Download triggers DownloadCustom`() = runTest {
-    testFlow(
-      flow = customDownloadViewModel.effects,
-      triggerAction = { customDownloadViewModel.actions.emit(ClickedDownload) },
-      assert = {
-        assertThat(awaitItem()).isEqualTo(setPreferredStorageWithMostSpace)
-        assertThat(awaitItem()).isEqualTo(downloadCustom)
-      }
-    )
+  internal fun `clicking Download triggers DownloadCustom`() = flakyTest {
+    runTest {
+      testFlow(
+        flow = customDownloadViewModel.effects,
+        triggerAction = { customDownloadViewModel.actions.emit(ClickedDownload) },
+        assert = {
+          assertThat(awaitItem()).isEqualTo(setPreferredStorageWithMostSpace)
+          assertThat(awaitItem()).isEqualTo(downloadCustom)
+        }
+      )
+    }
  }
 }

@ -248,3 +266,23 @@ suspend fun <T> TestScope.testFlow(
 }

 val TURBINE_TIMEOUT = 5000.toDuration(DurationUnit.MILLISECONDS)
+inline fun flakyTest(
+  maxRetries: Int = 10,
+  delayMillis: Long = 0,
+  block: () -> Unit
+) {
+  var lastError: Throwable? = null
+
+  repeat(maxRetries) { attempt ->
+    try {
+      block()
+      return
+    } catch (e: Throwable) {
+      lastError = e
+      println("Test attempt ${attempt + 1} failed: ${e.message}")
+      if (delayMillis > 0) Thread.sleep(delayMillis)
+    }
+  }
+
+  throw lastError ?: AssertionError("Test failed after $maxRetries attempts")
+}