Fix unit tests (#5947)

* move createLocale() method to companion object and add test dependency * use mockk() from Mockk library for mocking sealed classes * change method parameter to null-able String type * add null check for accessing property from unit tests * change method signature to match old method's signature It fixes the NullPointerException when running ImageProcessingUnitTest * Fix unresolved references and make properties public for unit tests * fix tests in UploadRepositoryUnitTest by making return type null-able
commons-app · Nov 22, 2024 · e070c5d · e070c5d
1 parent fe347c2
commit e070c5d
Show file tree

Hide file tree

Showing 14 changed files with 56 additions and 52 deletions.
diff --git a/app/build.gradle b/app/build.gradle
@@ -98,6 +98,7 @@ dependencies {
     testImplementation 'org.mockito:mockito-core:5.6.0'
     testImplementation "org.powermock:powermock-module-junit4:2.0.9"
     testImplementation "org.powermock:powermock-api-mockito2:2.0.9"
+    testImplementation("io.mockk:mockk:1.13.5")
 
     // Unit testing
     testImplementation 'junit:junit:4.13.2'

diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.kt b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.kt
@@ -124,7 +124,9 @@ class CategoryClient
                 }.map {
                     it
                         .filter { page ->
-                            !page.categoryInfo().isHidden
+                            // Null check is not redundant because some values could be null
+                            // for mocks when running unit tests
+                            page.categoryInfo()?.isHidden != true
                         }.map {
                             CategoryItem(
                                 it.title().replace(CATEGORY_PREFIX, ""),

diff --git a/app/src/main/java/fr/free/nrw/commons/recentlanguages/RecentLanguagesDao.kt b/app/src/main/java/fr/free/nrw/commons/recentlanguages/RecentLanguagesDao.kt
@@ -6,7 +6,6 @@ import android.content.ContentValues
 import android.database.Cursor
 import android.database.sqlite.SQLiteDatabase
 import android.os.RemoteException
-import java.util.ArrayList
 import javax.inject.Inject
 import javax.inject.Named
 import javax.inject.Provider
@@ -163,9 +162,9 @@ class RecentLanguagesDao @Inject constructor(
             COLUMN_CODE
         )
 
-        private const val DROP_TABLE_STATEMENT = "DROP TABLE IF EXISTS $TABLE_NAME"
+        const val DROP_TABLE_STATEMENT = "DROP TABLE IF EXISTS $TABLE_NAME"
 
-        private const val CREATE_TABLE_STATEMENT = "CREATE TABLE $TABLE_NAME (" +
+        const val CREATE_TABLE_STATEMENT = "CREATE TABLE $TABLE_NAME (" +
                 "$COLUMN_NAME STRING," +
                 "$COLUMN_CODE STRING PRIMARY KEY" +
                 ");"

diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.kt b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.kt
@@ -19,10 +19,10 @@ import fr.free.nrw.commons.upload.structure.depictions.DepictedItem
 import io.reactivex.Flowable
 import io.reactivex.Observable
 import io.reactivex.Single
+import timber.log.Timber
 import java.util.Locale
 import javax.inject.Inject
 import javax.inject.Singleton
-import timber.log.Timber
 
 /**
  * The repository class for UploadActivity
@@ -46,7 +46,7 @@ class UploadRepository @Inject constructor(
      *
      * @return
      */
-    fun buildContributions(): Observable<Contribution> {
+    fun buildContributions(): Observable<Contribution>? {
         return uploadModel.buildContributions()
     }
 
@@ -150,7 +150,7 @@ class UploadRepository @Inject constructor(
      *
      * @return
      */
-    fun getSelectedLicense(): String {
+    fun getSelectedLicense(): String? {
         return uploadModel.selectedLicense
     }
 
@@ -173,11 +173,11 @@ class UploadRepository @Inject constructor(
      * @return
      */
     fun preProcessImage(
-        uploadableFile: UploadableFile,
+        uploadableFile: UploadableFile?,
         place: Place?,
-        similarImageInterface: SimilarImageInterface,
+        similarImageInterface: SimilarImageInterface?,
         inAppPictureLocation: LatLng?
-    ): Observable<UploadItem> {
+    ): Observable<UploadItem>? {
         return uploadModel.preProcessImage(
             uploadableFile,
             place,
@@ -193,7 +193,7 @@ class UploadRepository @Inject constructor(
      * @param location Location of the image
      * @return Quality of UploadItem
      */
-    fun getImageQuality(uploadItem: UploadItem, location: LatLng?): Single<Int> {
+    fun getImageQuality(uploadItem: UploadItem, location: LatLng?): Single<Int>? {
         return uploadModel.getImageQuality(uploadItem, location)
     }
 
@@ -213,7 +213,7 @@ class UploadRepository @Inject constructor(
      * @param uploadItem UploadItem whose caption is to be checked
      * @return Quality of caption of the UploadItem
      */
-    fun getCaptionQuality(uploadItem: UploadItem): Single<Int> {
+    fun getCaptionQuality(uploadItem: UploadItem): Single<Int>? {
         return uploadModel.getCaptionQuality(uploadItem)
     }
 

diff --git a/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.kt b/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.kt
@@ -471,18 +471,20 @@ class SettingsFragment : PreferenceFragmentCompat() {
         editor.apply()
     }
 
-    /**
-     * Create Locale based on different types of language codes
-     * @param languageCode
-     * @return Locale and throws error for invalid language codes
-     */
-    fun createLocale(languageCode: String): Locale {
-        val parts = languageCode.split("-")
-        return when (parts.size) {
-            1 -> Locale(parts[0])
-            2 -> Locale(parts[0], parts[1])
-            3 -> Locale(parts[0], parts[1], parts[2])
-            else -> throw IllegalArgumentException("Invalid language code: $languageCode")
+    companion object {
+        /**
+         * Create Locale based on different types of language codes
+         * @param languageCode
+         * @return Locale and throws error for invalid language codes
+         */
+        fun createLocale(languageCode: String): Locale {
+            val parts = languageCode.split("-")
+            return when (parts.size) {
+                1 -> Locale(parts[0])
+                2 -> Locale(parts[0], parts[1])
+                3 -> Locale(parts[0], parts[1], parts[2])
+                else -> throw IllegalArgumentException("Invalid language code: $languageCode")
+            }
         }
     }
 

diff --git a/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt b/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt
@@ -16,7 +16,7 @@ class ImageUtilsWrapper @Inject constructor() {
 
     fun checkImageGeolocationIsDifferent(
         geolocationOfFileString: String,
-        latLng: LatLng
+        latLng: LatLng?
     ): Single<Int> {
         return Single.fromCallable {
             ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, latLng)

diff --git a/app/src/main/java/fr/free/nrw/commons/utils/MediaDataExtractorUtil.kt b/app/src/main/java/fr/free/nrw/commons/utils/MediaDataExtractorUtil.kt
@@ -1,9 +1,5 @@
 package fr.free.nrw.commons.utils
 
-import org.apache.commons.lang3.StringUtils
-
-import java.util.ArrayList
-
 object MediaDataExtractorUtil {
 
     /**
@@ -13,8 +9,8 @@ object MediaDataExtractorUtil {
      * @return
      */
     @JvmStatic
-    fun extractCategoriesFromList(source: String): List<String> {
-        if (source.isBlank()) {
+    fun extractCategoriesFromList(source: String?): List<String> {
+        if (source.isNullOrBlank()) {
             return emptyList()
         }
         val cats = source.split("|")

diff --git a/app/src/test/kotlin/fr/free/nrw/commons/auth/LoginActivityUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/auth/LoginActivityUnitTests.kt
@@ -16,6 +16,7 @@ import fr.free.nrw.commons.TestCommonsApplication
 import fr.free.nrw.commons.auth.login.LoginResult
 import fr.free.nrw.commons.createTestClient
 import fr.free.nrw.commons.kvstore.JsonKvStore
+import io.mockk.mockk
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
@@ -66,11 +67,11 @@ class LoginActivityUnitTests {
     @Mock
     private lateinit var account: Account
 
-    @Mock
     private lateinit var loginResult: LoginResult
 
     @Before
     fun setUp() {
+        loginResult = mockk()
         MockitoAnnotations.openMocks(this)
         OkHttpConnectionFactory.CLIENT = createTestClient()
         activity = Robolectric.buildActivity(LoginActivity::class.java).create().get()

diff --git a/app/src/test/kotlin/fr/free/nrw/commons/auth/SessionManagerUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/auth/SessionManagerUnitTests.kt
@@ -7,13 +7,14 @@ import androidx.test.core.app.ApplicationProvider
 import fr.free.nrw.commons.TestCommonsApplication
 import fr.free.nrw.commons.auth.login.LoginResult
 import fr.free.nrw.commons.kvstore.JsonKvStore
+import io.mockk.every
+import io.mockk.mockk
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
 import org.mockito.MockitoAnnotations
-import org.powermock.api.mockito.PowerMockito.`when`
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.Shadows.shadowOf
 import org.robolectric.annotation.Config
@@ -33,14 +34,14 @@ class SessionManagerUnitTests {
     @Mock
     private lateinit var defaultKvStore: JsonKvStore
 
-    @Mock
     private lateinit var loginResult: LoginResult
 
     @Mock
     private lateinit var context: Context
 
     @Before
     fun setUp() {
+        loginResult = mockk()
         MockitoAnnotations.openMocks(this)
         accountManager = AccountManager.get(ApplicationProvider.getApplicationContext())
         shadowOf(accountManager).addAccount(account)
@@ -68,8 +69,8 @@ class SessionManagerUnitTests {
     @Test
     @Throws(Exception::class)
     fun testUpdateAccount() {
-        `when`(loginResult.userName).thenReturn("username")
-        `when`(loginResult.password).thenReturn("password")
+        every { loginResult.userName } returns "username"
+        every { loginResult.password } returns "password"
         val method: Method =
             SessionManager::class.java.getDeclaredMethod(
                 "updateAccount",

diff --git a/app/src/test/kotlin/fr/free/nrw/commons/recentlanguages/RecentLanguagesDaoUnitTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/recentlanguages/RecentLanguagesDaoUnitTest.kt
@@ -85,7 +85,7 @@ class RecentLanguagesDaoUnitTest {
         whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull()))
             .thenReturn(createCursor(14))
 
-        val result = testObject.recentLanguages
+        val result = testObject.getRecentLanguages()
 
         Assert.assertEquals(14, (result.size))
     }
@@ -95,20 +95,20 @@ class RecentLanguagesDaoUnitTest {
         whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenThrow(
             RemoteException(""),
         )
-        testObject.recentLanguages
+        testObject.getRecentLanguages()
     }
 
     @Test
     fun getGetRecentLanguagesReturnsEmptyList_emptyCursor() {
         whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull()))
             .thenReturn(createCursor(0))
-        Assert.assertTrue(testObject.recentLanguages.isEmpty())
+        Assert.assertTrue(testObject.getRecentLanguages().isEmpty())
     }
 
     @Test
     fun getGetRecentLanguagesReturnsEmptyList_nullCursor() {
         whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(null)
-        Assert.assertTrue(testObject.recentLanguages.isEmpty())
+        Assert.assertTrue(testObject.getRecentLanguages().isEmpty())
     }
 
     @Test
@@ -117,7 +117,7 @@ class RecentLanguagesDaoUnitTest {
         whenever(client.query(any(), any(), anyOrNull(), any(), anyOrNull())).thenReturn(mockCursor)
         whenever(mockCursor.moveToFirst()).thenReturn(false)
 
-        testObject.recentLanguages
+        testObject.getRecentLanguages()
 
         verify(mockCursor).close()
     }

diff --git a/app/src/test/kotlin/fr/free/nrw/commons/settings/SettingsFragmentUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/settings/SettingsFragmentUnitTests.kt
@@ -19,7 +19,7 @@ import fr.free.nrw.commons.TestCommonsApplication
 import fr.free.nrw.commons.recentlanguages.Language
 import fr.free.nrw.commons.recentlanguages.RecentLanguagesAdapter
 import fr.free.nrw.commons.recentlanguages.RecentLanguagesDao
-import fr.free.nrw.commons.settings.SettingsFragment.createLocale
+import fr.free.nrw.commons.settings.SettingsFragment.Companion.createLocale
 import org.junit.Assert
 import org.junit.Assert.assertEquals
 import org.junit.Before
@@ -156,14 +156,14 @@ class SettingsFragmentUnitTests {
             )
         method.isAccessible = true
         method.invoke(fragment, "appUiDefaultLanguagePref")
-        verify(recentLanguagesDao, times(1)).recentLanguages
+        verify(recentLanguagesDao, times(1)).getRecentLanguages()
     }
 
     @Test
     @Throws(Exception::class)
     fun `Test prepareAppLanguages when recently used languages is not empty`() {
         Shadows.shadowOf(Looper.getMainLooper()).idle()
-        whenever(recentLanguagesDao.recentLanguages)
+        whenever(recentLanguagesDao.getRecentLanguages())
             .thenReturn(
                 mutableListOf(
                     Language("English", "en"),
@@ -181,7 +181,7 @@ class SettingsFragmentUnitTests {
             )
         method.isAccessible = true
         method.invoke(fragment, "appUiDefaultLanguagePref")
-        verify(recentLanguagesDao, times(2)).recentLanguages
+        verify(recentLanguagesDao, times(2)).getRecentLanguages()
     }
 
     @Test

diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt
@@ -131,7 +131,7 @@ class UploadMediaPresenterTest {
      */
     @Test
     fun getImageQualityTest() {
-        whenever(repository.uploads).thenReturn(listOf(uploadItem))
+        whenever(repository.getUploads()).thenReturn(listOf(uploadItem))
         whenever(repository.getImageQuality(uploadItem, location))
             .thenReturn(testSingleImageResult)
         whenever(uploadItem.imageQuality).thenReturn(0)
@@ -149,7 +149,7 @@ class UploadMediaPresenterTest {
      */
     @Test
     fun `get ImageQuality Test while coordinates equals to null`() {
-        whenever(repository.uploads).thenReturn(listOf(uploadItem))
+        whenever(repository.getUploads()).thenReturn(listOf(uploadItem))
         whenever(repository.getImageQuality(uploadItem, location))
             .thenReturn(testSingleImageResult)
         whenever(uploadItem.imageQuality).thenReturn(0)
@@ -225,7 +225,7 @@ class UploadMediaPresenterTest {
      */
     @Test
     fun fetchImageAndTitleTest() {
-        whenever(repository.uploads).thenReturn(listOf(uploadItem))
+        whenever(repository.getUploads()).thenReturn(listOf(uploadItem))
         whenever(repository.getUploadItem(ArgumentMatchers.anyInt()))
             .thenReturn(uploadItem)
         whenever(uploadItem.uploadMediaDetails).thenReturn(listOf())

diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadRepositoryUnitTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadRepositoryUnitTest.kt
@@ -15,6 +15,7 @@ import fr.free.nrw.commons.repository.UploadRepository
 import fr.free.nrw.commons.upload.structure.depictions.DepictModel
 import fr.free.nrw.commons.upload.structure.depictions.DepictedItem
 import io.reactivex.Completable
+import io.reactivex.Observable
 import io.reactivex.Single
 import org.junit.Before
 import org.junit.Test
@@ -196,7 +197,7 @@ class UploadRepositoryUnitTest {
     fun testGetCaptionQuality() {
         assertEquals(
             repository.getCaptionQuality(uploadItem),
-            uploadModel.getCaptionQuality(uploadItem),
+            uploadModel.getCaptionQuality(uploadItem)
         )
     }
 
@@ -386,7 +387,8 @@ class UploadRepositoryUnitTest {
     fun testGetCategories() {
         assertEquals(
             repository.getCategories(listOf("Test")),
-            categoriesModel.getCategoriesByName(mutableListOf("Test")),
+            categoriesModel.getCategoriesByName(mutableListOf("Test"))
+                ?: Observable.empty<List<CategoryItem>>()
         )
     }
 }
diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/structure/depictions/DepictedItemTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/structure/depictions/DepictedItemTest.kt
@@ -1,10 +1,10 @@
 package fr.free.nrw.commons.upload.structure.depictions
 
-import com.nhaarman.mockitokotlin2.mock
 import depictedItem
 import entity
 import entityId
 import fr.free.nrw.commons.wikidata.WikidataProperties
+import io.mockk.mockk
 import org.junit.Assert
 import org.junit.Test
 import place
@@ -53,7 +53,7 @@ class DepictedItemTest {
                 entity(
                     statements =
                         mapOf(
-                            WikidataProperties.IMAGE.propertyName to listOf(statement(snak(dataValue = mock()))),
+                            WikidataProperties.IMAGE.propertyName to listOf(statement(snak(dataValue = mockk()))),
                         ),
                 ),
             ).imageUrl,