From 439dd4ff780fe9501be1a066ea1f99d7884aae2a Mon Sep 17 00:00:00 2001 From: Chris Narkiewicz Date: Tue, 3 Mar 2020 23:13:21 +0000 Subject: [PATCH] Removed MigrationManager Robolectric tests Robolectric tests are removed as we don't want to emulate platform. Renamed instrumentation tests to match expected naming. Updated detekt rule to allow human readable test function names in kt files. This naming convention was adopted in test code to enable human-readable test reports, but DEX does not support spaces in names. Underscore _ is a good replacement providnig similar level of readability. Signed-off-by: Chris Narkiewicz --- build.gradle | 1 - detekt.yml | 1 + .../migrations/MigrationsManagerTest.kt} | 30 +- .../migrations/MockSharedPreferences.kt | 0 .../migrations/MockSharedPreferencesTest.kt} | 10 +- .../android/MockSharedPreferences.kt | 104 ------- .../android/TestMigrationsManager.kt | 257 ------------------ .../android/TestMockSharedPreferences.kt | 77 ------ 8 files changed, 23 insertions(+), 457 deletions(-) rename src/{test/java/com/nextcloud/client/migrations/TestMigrationsManager.kt => androidTest/java/com/nextcloud/client/migrations/MigrationsManagerTest.kt} (93%) rename src/{test => androidTest}/java/com/nextcloud/client/migrations/MockSharedPreferences.kt (100%) rename src/{test/java/com/nextcloud/client/migrations/TestMockSharedPreferences.kt => androidTest/java/com/nextcloud/client/migrations/MockSharedPreferencesTest.kt} (93%) delete mode 100644 src/androidTest/java/com/nextcloud/client/migrations/android/MockSharedPreferences.kt delete mode 100644 src/androidTest/java/com/nextcloud/client/migrations/android/TestMigrationsManager.kt delete mode 100644 src/androidTest/java/com/nextcloud/client/migrations/android/TestMockSharedPreferences.kt diff --git a/build.gradle b/build.gradle index 4450d5646d..1b94d1b297 100644 --- a/build.gradle +++ b/build.gradle @@ -387,7 +387,6 @@ dependencies { // androidJacocoAnt "org.jacoco:org.jacoco.agent:${jacocoVersion}" implementation "com.github.stateless4j:stateless4j:2.6.0" - testImplementation "org.robolectric:robolectric:4.3.1" androidTestImplementation "com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0" androidTestImplementation "org.mockito:mockito-android:3.3.0" } diff --git a/detekt.yml b/detekt.yml index 1835fe1282..16a16e86f1 100644 --- a/detekt.yml +++ b/detekt.yml @@ -265,6 +265,7 @@ naming: functionPattern: '^([a-z$][a-zA-Z$0-9]*)|(`.*`)$' excludeClassPattern: '$^' ignoreOverridden: true + excludes: "**/*Test.kt" FunctionParameterNaming: active: true parameterPattern: '[a-z][A-Za-z0-9]*' diff --git a/src/test/java/com/nextcloud/client/migrations/TestMigrationsManager.kt b/src/androidTest/java/com/nextcloud/client/migrations/MigrationsManagerTest.kt similarity index 93% rename from src/test/java/com/nextcloud/client/migrations/TestMigrationsManager.kt rename to src/androidTest/java/com/nextcloud/client/migrations/MigrationsManagerTest.kt index 9bdf499459..6a2e59561a 100644 --- a/src/test/java/com/nextcloud/client/migrations/TestMigrationsManager.kt +++ b/src/androidTest/java/com/nextcloud/client/migrations/MigrationsManagerTest.kt @@ -19,6 +19,7 @@ */ package com.nextcloud.client.migrations +import androidx.test.annotation.UiThreadTest import com.nextcloud.client.account.UserAccountManager import com.nextcloud.client.appinfo.AppInfo import com.nextcloud.client.core.ManualAsyncRunner @@ -31,15 +32,12 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test -import org.junit.runner.RunWith import org.mockito.Mock import org.mockito.MockitoAnnotations -import org.robolectric.RobolectricTestRunner import java.lang.RuntimeException import java.util.LinkedHashSet -@RunWith(RobolectricTestRunner::class) -class TestMigrationsManager { +class MigrationsManagerTest { companion object { const val OLD_APP_VERSION = 41 @@ -83,7 +81,7 @@ class TestMigrationsManager { } @Test - fun `inital status is unknown`() { + fun inital_status_is_unknown() { // GIVEN // migration manager has not been used yets @@ -93,7 +91,7 @@ class TestMigrationsManager { } @Test - fun `applied migrations are returned in order`() { + fun applied_migrations_are_returned_in_order() { // GIVEN // some migrations are marked as applied // migration ids are stored in random order @@ -117,7 +115,7 @@ class TestMigrationsManager { @Test @Suppress("MagicNumber") - fun `registering new applied migration preserves old ids`() { + fun registering_new_applied_migration_preserves_old_ids() { // WHEN // some applied migrations are registered val appliedMigrationIds = setOf("0", "1", "2") @@ -135,7 +133,8 @@ class TestMigrationsManager { } @Test - fun `migrations are scheduled on background thread`() { + @UiThreadTest + fun migrations_are_scheduled_on_background_thread() { // GIVEN // migrations can be applied assertEquals(0, migrationsManager.getAppliedMigrations().size) @@ -153,7 +152,8 @@ class TestMigrationsManager { } @Test - fun `applied migrations are recorded`() { + @UiThreadTest + fun applied_migrations_are_recorded() { // GIVEN // no migrations are applied yet // current app version is newer then last recorded migrated version @@ -178,7 +178,8 @@ class TestMigrationsManager { } @Test - fun `migration error is recorded`() { + @UiThreadTest + fun migration_error_is_recorded() { // GIVEN // no migrations applied yet @@ -208,7 +209,8 @@ class TestMigrationsManager { } @Test - fun `migrations are not run if already run for an app version`() { + @UiThreadTest + fun migrations_are_not_run_if_already_run_for_an_app_version() { // GIVEN // migrations were already run for the current app version whenever(appInfo.versionCode).thenReturn(NEW_APP_VERSION) @@ -229,7 +231,8 @@ class TestMigrationsManager { } @Test - fun `new app version is marked as migrated if no new migrations are available`() { + @UiThreadTest + fun new_app_version_is_marked_as_migrated_if_no_new_migrations_are_available() { // GIVEN // migrations were applied in previous version // new version has no new migrations @@ -253,7 +256,8 @@ class TestMigrationsManager { } @Test - fun `optional migration failure does not trigger a migration failure`() { + @UiThreadTest + fun optional_migration_failure_does_not_trigger_a_migration_failure() { // GIVEN // pending migrations // mandatory migrations are passing diff --git a/src/test/java/com/nextcloud/client/migrations/MockSharedPreferences.kt b/src/androidTest/java/com/nextcloud/client/migrations/MockSharedPreferences.kt similarity index 100% rename from src/test/java/com/nextcloud/client/migrations/MockSharedPreferences.kt rename to src/androidTest/java/com/nextcloud/client/migrations/MockSharedPreferences.kt diff --git a/src/test/java/com/nextcloud/client/migrations/TestMockSharedPreferences.kt b/src/androidTest/java/com/nextcloud/client/migrations/MockSharedPreferencesTest.kt similarity index 93% rename from src/test/java/com/nextcloud/client/migrations/TestMockSharedPreferences.kt rename to src/androidTest/java/com/nextcloud/client/migrations/MockSharedPreferencesTest.kt index d0156b341e..0598bcea8c 100644 --- a/src/test/java/com/nextcloud/client/migrations/TestMockSharedPreferences.kt +++ b/src/androidTest/java/com/nextcloud/client/migrations/MockSharedPreferencesTest.kt @@ -27,7 +27,7 @@ import org.junit.Assert.assertNotSame import org.junit.Assert.assertTrue @Suppress("MagicNumber") -class TestMockSharedPreferences { +class MockSharedPreferencesTest { private lateinit var mock: MockSharedPreferences @@ -37,7 +37,7 @@ class TestMockSharedPreferences { } @Test - fun `get set string set`() { + fun getSetStringSet() { val value = setOf("alpha", "bravo", "charlie") mock.edit().putStringSet("key", value).apply() val copy = mock.getStringSet("key", mutableSetOf()) @@ -46,7 +46,7 @@ class TestMockSharedPreferences { } @Test - fun `get set int`() { + fun getSetInt() { val value = 42 val editor = mock.edit() editor.putInt("key", value) @@ -56,7 +56,7 @@ class TestMockSharedPreferences { } @Test - fun `get set boolean`() { + fun getSetBoolean() { val value = true val editor = mock.edit() editor.putBoolean("key", value) @@ -66,7 +66,7 @@ class TestMockSharedPreferences { } @Test - fun `get set string`() { + fun getSetString() { val value = "a value" val editor = mock.edit() editor.putString("key", value) diff --git a/src/androidTest/java/com/nextcloud/client/migrations/android/MockSharedPreferences.kt b/src/androidTest/java/com/nextcloud/client/migrations/android/MockSharedPreferences.kt deleted file mode 100644 index 81835edca9..0000000000 --- a/src/androidTest/java/com/nextcloud/client/migrations/android/MockSharedPreferences.kt +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Nextcloud Android client application - * - * @author Chris Narkiewicz - * Copyright (C) 2020 Chris Narkiewicz - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ -package com.nextcloud.client.migrations.android - -import android.content.SharedPreferences -import java.lang.UnsupportedOperationException -import java.util.TreeMap - -@Suppress("TooManyFunctions") -class MockSharedPreferences : SharedPreferences { - - class MockEditor(val store: MutableMap) : SharedPreferences.Editor { - - val editorStore: MutableMap = TreeMap() - - override fun clear(): SharedPreferences.Editor = throw UnsupportedOperationException() - - override fun putLong(key: String?, value: Long): SharedPreferences.Editor = - throw UnsupportedOperationException() - - override fun putInt(key: String?, value: Int): SharedPreferences.Editor { - editorStore.put(key, value) - return this - } - - override fun remove(key: String?): SharedPreferences.Editor = throw UnsupportedOperationException() - - override fun putBoolean(key: String?, value: Boolean): SharedPreferences.Editor { - editorStore.put(key, value) - return this - } - - override fun putStringSet(key: String?, values: MutableSet?): SharedPreferences.Editor { - editorStore.put(key, values?.toMutableSet()) - return this - } - - override fun commit(): Boolean = true - - override fun putFloat(key: String?, value: Float): SharedPreferences.Editor = - throw UnsupportedOperationException() - - override fun apply() = store.putAll(editorStore) - - override fun putString(key: String?, value: String?): SharedPreferences.Editor { - editorStore.put(key, value) - return this - } - } - - val store: MutableMap = TreeMap() - - override fun contains(key: String?): Boolean = store.containsKey(key) - override fun getBoolean(key: String?, defValue: Boolean): Boolean = store.getOrDefault(key, defValue) as Boolean - - override fun unregisterOnSharedPreferenceChangeListener( - listener: SharedPreferences.OnSharedPreferenceChangeListener? - ) = throw UnsupportedOperationException() - - override fun getInt(key: String?, defValue: Int): Int = store.getOrDefault(key, defValue) as Int - - override fun getAll(): MutableMap { - throw UnsupportedOperationException() - } - - override fun edit(): SharedPreferences.Editor { - return MockEditor(store) - } - - override fun getLong(key: String?, defValue: Long): Long { - throw UnsupportedOperationException() - } - - override fun getFloat(key: String?, defValue: Float): Float { - throw UnsupportedOperationException() - } - - override fun getStringSet(key: String?, defValues: MutableSet?): MutableSet? { - return store.getOrDefault(key, defValues) as MutableSet? - } - - override fun registerOnSharedPreferenceChangeListener( - listener: SharedPreferences.OnSharedPreferenceChangeListener? - ) = throw UnsupportedOperationException() - - override fun getString(key: String?, defValue: String?): String? = store.getOrDefault(key, defValue) as String? -} diff --git a/src/androidTest/java/com/nextcloud/client/migrations/android/TestMigrationsManager.kt b/src/androidTest/java/com/nextcloud/client/migrations/android/TestMigrationsManager.kt deleted file mode 100644 index e06bd44ee0..0000000000 --- a/src/androidTest/java/com/nextcloud/client/migrations/android/TestMigrationsManager.kt +++ /dev/null @@ -1,257 +0,0 @@ -/* - * Nextcloud Android client application - * - * @author Chris Narkiewicz - * Copyright (C) 2020 Chris Narkiewicz - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ -package com.nextcloud.client.migrations.android - -import androidx.test.annotation.UiThreadTest -import com.nextcloud.client.account.UserAccountManager -import com.nextcloud.client.appinfo.AppInfo -import com.nextcloud.client.core.ManualAsyncRunner -import com.nextcloud.client.migrations.Migrations -import com.nextcloud.client.migrations.MigrationsManager -import com.nextcloud.client.migrations.MigrationsManagerImpl -import com.nhaarman.mockitokotlin2.mock -import com.nhaarman.mockitokotlin2.never -import com.nhaarman.mockitokotlin2.verify -import com.nhaarman.mockitokotlin2.whenever -import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Test -import org.mockito.Mock -import org.mockito.MockitoAnnotations -import java.lang.RuntimeException -import java.util.LinkedHashSet - -@Suppress("FunctionNaming") -class TestMigrationsManager { - - companion object { - const val OLD_APP_VERSION = 41 - const val NEW_APP_VERSION = 42 - } - - lateinit var migrations: List - - @Mock - lateinit var appInfo: AppInfo - - lateinit var migrationsDb: MockSharedPreferences - - @Mock - lateinit var userAccountManager: UserAccountManager - - lateinit var asyncRunner: ManualAsyncRunner - - internal lateinit var migrationsManager: MigrationsManagerImpl - - @Before - fun setUp() { - MockitoAnnotations.initMocks(this) - val migrationStep1: Runnable = mock() - val migrationStep2: Runnable = mock() - migrations = listOf( - Migrations.Step(0, "first migration", migrationStep1), - Migrations.Step(1, "second migration", migrationStep2) - ) - asyncRunner = ManualAsyncRunner() - migrationsDb = MockSharedPreferences() - whenever(appInfo.versionCode).thenReturn(NEW_APP_VERSION) - migrationsManager = MigrationsManagerImpl( - appInfo = appInfo, - migrationsDb = migrationsDb, - asyncRunner = asyncRunner, - migrations = migrations - ) - } - - @Test - fun inital_status_is_unknown() { - // GIVEN - // migration manager has not been used yets - - // THEN - // status is not set - assertEquals(MigrationsManager.Status.UNKNOWN, migrationsManager.status.value) - } - - @Test - fun applied_migrations_are_returned_in_order() { - // GIVEN - // some migrations are marked as applied - // migration ids are stored in random order - val storedMigrationIds = LinkedHashSet() - storedMigrationIds.apply { - add("3") - add("0") - add("2") - add("1") - } - migrationsDb.store.put(MigrationsManagerImpl.DB_KEY_APPLIED_MIGRATIONS, storedMigrationIds) - - // WHEN - // applied migration ids are retrieved - val ids = migrationsManager.getAppliedMigrations() - - // THEN - // returned list is sorted - assertEquals(ids, ids.sorted()) - } - - @Test - @Suppress("MagicNumber") - fun registering_new_applied_migration_preserves_old_ids() { - // WHEN - // some applied migrations are registered - val appliedMigrationIds = setOf("0", "1", "2") - migrationsDb.store.put(MigrationsManagerImpl.DB_KEY_APPLIED_MIGRATIONS, appliedMigrationIds) - - // WHEN - // new set of migration ids are registered - // some ids are added again - migrationsManager.addAppliedMigration(2, 3, 4) - - // THEN - // new ids are appended to set of existing ids - val expectedIds = setOf("0", "1", "2", "3", "4") - assertEquals(expectedIds, migrationsDb.store.get(MigrationsManagerImpl.DB_KEY_APPLIED_MIGRATIONS)) - } - - @Test - @UiThreadTest - fun migrations_are_scheduled_on_background_thread() { - // GIVEN - // migrations can be applied - assertEquals(0, migrationsManager.getAppliedMigrations().size) - - // WHEN - // migration is started - val count = migrationsManager.startMigration() - - // THEN - // all migrations are scheduled on background thread - // single task is scheduled - assertEquals(migrations.size, count) - assertEquals(1, asyncRunner.size) - assertEquals(MigrationsManager.Status.RUNNING, migrationsManager.status.value) - } - - @Test - @UiThreadTest - fun applied_migrations_are_recorded() { - // GIVEN - // no migrations are applied yet - // current app version is newer then last recorded migrated version - whenever(appInfo.versionCode).thenReturn(NEW_APP_VERSION) - migrationsDb.store.put(MigrationsManagerImpl.DB_KEY_LAST_MIGRATED_VERSION, OLD_APP_VERSION) - - // WHEN - // migration is run - whenever(userAccountManager.migrateUserId()).thenReturn(true) - val count = migrationsManager.startMigration() - assertTrue(asyncRunner.runOne()) - - // THEN - // total migrations count is returned - // migration functions are called - // applied migrations are recorded - // new app version code is recorded - assertEquals(migrations.size, count) - assertEquals(setOf("0", "1"), migrationsDb.store.get(MigrationsManagerImpl.DB_KEY_APPLIED_MIGRATIONS)) - assertEquals(NEW_APP_VERSION, migrationsDb.store.get(MigrationsManagerImpl.DB_KEY_LAST_MIGRATED_VERSION)) - } - - @Test - @UiThreadTest - fun migration_error_is_recorded() { - // GIVEN - // no migrations applied yet - - // WHEN - // migrations are applied - // one migration throws - val lastMigration = migrations.last() - val errorMessage = "error message" - whenever(lastMigration.function.run()).thenThrow(RuntimeException(errorMessage)) - migrationsManager.startMigration() - assertTrue(asyncRunner.runOne()) - - // THEN - // failure is marked in the migration db - // failure message is recorded - // failed migration id is recorded - assertEquals(MigrationsManager.Status.FAILED, migrationsManager.status.value) - assertTrue(migrationsDb.getBoolean(MigrationsManagerImpl.DB_KEY_FAILED, false)) - assertEquals( - errorMessage, - migrationsDb.getString(MigrationsManagerImpl.DB_KEY_FAILED_MIGRATION_ERROR_MESSAGE, "") - ) - assertEquals( - lastMigration.id, - migrationsDb.getInt(MigrationsManagerImpl.DB_KEY_FAILED_MIGRATION_ID, -1) - ) - } - - @Test - @UiThreadTest - fun migrations_are_not_run_if_already_run_for_an_app_version() { - // GIVEN - // migrations were already run for the current app version - whenever(appInfo.versionCode).thenReturn(NEW_APP_VERSION) - migrationsDb.store.put(MigrationsManagerImpl.DB_KEY_LAST_MIGRATED_VERSION, NEW_APP_VERSION) - - // WHEN - // app is migrated again - val migrationCount = migrationsManager.startMigration() - - // THEN - // migration processing is skipped entirely - // status is set to applied - assertEquals(0, migrationCount) - migrations.forEach { - verify(it.function, never()).run() - } - assertEquals(MigrationsManager.Status.APPLIED, migrationsManager.status.value) - } - - @Test - @UiThreadTest - fun new_app_version_is_marked_as_migrated_if_no_new_migrations_are_available() { - // GIVEN - // migrations were applied in previous version - // new version has no new migrations - whenever(appInfo.versionCode).thenReturn(NEW_APP_VERSION) - migrationsDb.store.put(MigrationsManagerImpl.DB_KEY_LAST_MIGRATED_VERSION, OLD_APP_VERSION) - val applied = migrations.map { it.id.toString() }.toSet() - migrationsDb.store.put(MigrationsManagerImpl.DB_KEY_APPLIED_MIGRATIONS, applied) - - // WHEN - // migration is started - val startedCount = migrationsManager.startMigration() - - // THEN - // no new migrations are run - // new version is marked as migrated - assertEquals(0, startedCount) - assertEquals( - NEW_APP_VERSION, - migrationsDb.getInt(MigrationsManagerImpl.DB_KEY_LAST_MIGRATED_VERSION, -1) - ) - } -} diff --git a/src/androidTest/java/com/nextcloud/client/migrations/android/TestMockSharedPreferences.kt b/src/androidTest/java/com/nextcloud/client/migrations/android/TestMockSharedPreferences.kt deleted file mode 100644 index 659eac6a4f..0000000000 --- a/src/androidTest/java/com/nextcloud/client/migrations/android/TestMockSharedPreferences.kt +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Nextcloud Android client application - * - * @author Chris Narkiewicz - * Copyright (C) 2020 Chris Narkiewicz - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ -package com.nextcloud.client.migrations.android - -import org.junit.Before -import org.junit.Test -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertNotSame -import org.junit.Assert.assertTrue - -@Suppress("MagicNumber", "FunctionNaming") -class TestMockSharedPreferences { - - private lateinit var mock: MockSharedPreferences - - @Before - fun setUp() { - mock = MockSharedPreferences() - } - - @Test - fun get_set_string_set() { - val value = setOf("alpha", "bravo", "charlie") - mock.edit().putStringSet("key", value).apply() - val copy = mock.getStringSet("key", mutableSetOf()) - assertNotSame(value, copy) - assertEquals(value, copy) - } - - @Test - fun get_set_int() { - val value = 42 - val editor = mock.edit() - editor.putInt("key", value) - assertEquals(100, mock.getInt("key", 100)) - editor.apply() - assertEquals(42, mock.getInt("key", 100)) - } - - @Test - fun get_set_boolean() { - val value = true - val editor = mock.edit() - editor.putBoolean("key", value) - assertFalse(mock.getBoolean("key", false)) - editor.apply() - assertTrue(mock.getBoolean("key", false)) - } - - @Test - fun get_set_string() { - val value = "a value" - val editor = mock.edit() - editor.putString("key", value) - assertEquals("default", mock.getString("key", "default")) - editor.apply() - assertEquals("a value", mock.getString("key", "default")) - } -}