Fix unit tests (commons-app#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

Author: rohit9625

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'

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, ""),

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" +
                 ");"

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)
     }
 

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")
+            }
         }
     }
 

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)

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("|")

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()

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",

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()
     }