From 5f718553844acb0c92c691ca7681b6b9964b3310 Mon Sep 17 00:00:00 2001 From: Jason LeBrun Date: Tue, 3 Mar 2020 08:29:52 -0800 Subject: [PATCH] Simplify service test pattern * Make a simple test lifecycle registry, instead of creating empty testactivity * Remove use of `runBlockingTest`: according to https://github.com/Kotlin/kotlinx.coroutines/issues/1222, if the test results in coroutines being finished on other threads, it's possible to receive "This job has not yet completed" exceptions, even though the jobs were properly terminated. Since we don't need the delay-skipping properties of `runBlockingTest`, I think it's OK to simply using `runBlocking`, instead. --- .../handle/AndroidHandleManagerTest.kt | 214 ++++++++---------- .../storage/handle/AndroidManifest.xml | 6 - javatests/arcs/android/storage/handle/BUILD | 13 -- .../android/storage/handle/TestActivity.kt | 5 - 4 files changed, 98 insertions(+), 140 deletions(-) delete mode 100644 javatests/arcs/android/storage/handle/TestActivity.kt diff --git a/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt b/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt index 2aa9512e07c..be09416a7ae 100644 --- a/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt +++ b/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt @@ -2,6 +2,8 @@ package arcs.android.storage.handle import android.app.Application import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry import androidx.test.core.app.ActivityScenario import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -26,9 +28,10 @@ import arcs.sdk.android.storage.service.DefaultConnectionFactory import arcs.sdk.android.storage.service.testutil.TestBindingDelegate import com.google.common.truth.Truth.assertThat import com.nhaarman.mockitokotlin2.mock +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.test.TestCoroutineScope import kotlinx.coroutines.test.runBlockingTest import org.junit.Before import org.junit.Test @@ -39,27 +42,32 @@ import org.mockito.Mockito.verify @Suppress("EXPERIMENTAL_API_USAGE") @RunWith(AndroidJUnit4::class) -class AndroidHandleManagerTest { +class AndroidHandleManagerTest : LifecycleOwner { + private lateinit var lifecycle: LifecycleRegistry + override fun getLifecycle() = lifecycle + private lateinit var app: Application + private lateinit var handleManager: HandleManager + val entity1 = RawEntity( "entity1", - singletons=mapOf( + singletons = mapOf( "name" to "Jason".toReferencable(), "age" to 21.toReferencable(), "is_cool" to false.toReferencable() ), - collections=emptyMap() + collections = emptyMap() ) val entity2 = RawEntity( "entity2", - singletons=mapOf( + singletons = mapOf( "name" to "Jason".toReferencable(), "age" to 22.toReferencable(), "is_cool" to true.toReferencable() ), - collections=emptyMap() + collections = emptyMap() ) private val schema = Schema( @@ -88,140 +96,114 @@ class AndroidHandleManagerTest { @Before fun setUp() { RamDisk.clear() + lifecycle = LifecycleRegistry(this).apply { + setCurrentState(Lifecycle.State.CREATED) + setCurrentState(Lifecycle.State.STARTED) + setCurrentState(Lifecycle.State.RESUMED) + } app = ApplicationProvider.getApplicationContext() - app.setTheme(R.style.Theme_AppCompat); // Initialize WorkManager for instrumentation tests. WorkManagerTestInitHelper.initializeTestWorkManager(app) - } - fun handleManagerTest( - block: suspend TestCoroutineScope.(HandleManager) -> Unit - ) = runBlockingTest { - val scenario = ActivityScenario.launch(TestActivity::class.java) - - scenario.moveToState(Lifecycle.State.STARTED) - - val activityJob = launch { - scenario.onActivity { activity -> - val hf = AndroidHandleManager( - lifecycle = activity.lifecycle, - context = activity, - connectionFactory = DefaultConnectionFactory(activity, TestBindingDelegate(app)), - coroutineContext = coroutineContext - ) - runBlocking { - this@runBlockingTest.block(hf) - } - scenario.close() - } - } - - activityJob.join() + handleManager = AndroidHandleManager( + lifecycle = lifecycle, + context = app, + connectionFactory = DefaultConnectionFactory(app, TestBindingDelegate(app)) + ) } @Test - fun testCreateSingletonHandle() = runBlockingTest { - handleManagerTest { hm -> - val singletonHandle = hm.singletonHandle(singletonKey, schema) - singletonHandle.store(entity1) - - // Now read back from a different handle - val readbackHandle = hm.singletonHandle(singletonKey, schema) - val readBack = readbackHandle.fetch() - assertThat(readBack).isEqualTo(entity1) - } + fun singleton_writeAndReadback() = runBlocking { + val singletonHandle = handleManager.singletonHandle(singletonKey, schema) + singletonHandle.store(entity1) + + // Now read back from a different handle + val readbackHandle = handleManager.singletonHandle(singletonKey, schema) + val readBack = readbackHandle.fetch() + assertThat(readBack).isEqualTo(entity1) + } @Test - fun testCreateSetHandle() = runBlockingTest { - handleManagerTest { hm -> - val setHandle = hm.setHandle(setKey, schema) - setHandle.store(entity1) - setHandle.store(entity2) - - // Now read back from a different handle - val secondHandle = hm.setHandle(setKey, schema) - val readBack = secondHandle.fetchAll() - assertThat(readBack).containsExactly(entity1, entity2) - } + fun set_writeAndReadback() = runBlocking { + val setHandle = handleManager.setHandle(setKey, schema) + setHandle.store(entity1) + setHandle.store(entity2) + + // Now read back from a different handle + val secondHandle = handleManager.setHandle(setKey, schema) + val readBack = secondHandle.fetchAll() + assertThat(readBack).containsExactly(entity1, entity2) } private fun testMapForKey(key: StorageKey) = VersionMap(key.toKeyString() to 1) @Test - fun testSetHandleOnUpdate() = runBlockingTest { - handleManagerTest { hm -> - val testCallback1 = mock>() - val testCallback2 = mock>() - val firstHandle = hm.setHandle(setKey, schema, testCallback1) - val secondHandle = hm.setHandle(setKey, schema, testCallback2) - - val expectedAdd = CrdtSet.Operation.Add( - setKey.toKeyString(), - testMapForKey(setKey), - entity1 - ) - secondHandle.store(entity1) - verify(testCallback1, times(1)).onUpdate(firstHandle, expectedAdd) - verify(testCallback2, times(1)).onUpdate(secondHandle, expectedAdd) - - firstHandle.remove(entity1) - val expectedRemove = CrdtSet.Operation.Remove( - setKey.toKeyString(), - testMapForKey(setKey), - entity1 - ) - verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove) - verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove) - } + fun set_onHandleUpdate() = runBlocking { + val testCallback1 = mock>() + val testCallback2 = mock>() + val firstHandle = handleManager.setHandle(setKey, schema, testCallback1) + val secondHandle = handleManager.setHandle(setKey, schema, testCallback2) + + val expectedAdd = CrdtSet.Operation.Add( + setKey.toKeyString(), + testMapForKey(setKey), + entity1 + ) + secondHandle.store(entity1) + verify(testCallback1, times(1)).onUpdate(firstHandle, expectedAdd) + verify(testCallback2, times(1)).onUpdate(secondHandle, expectedAdd) + + firstHandle.remove(entity1) + val expectedRemove = CrdtSet.Operation.Remove( + setKey.toKeyString(), + testMapForKey(setKey), + entity1 + ) + verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove) + verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove) } @Test - fun testSingletonHandleOnUpdate() = runBlockingTest { - handleManagerTest { hm -> - val testCallback1 = mock>() - val testCallback2 = mock>() - val firstHandle = hm.singletonHandle(singletonKey, schema, testCallback1) - val secondHandle = hm.singletonHandle(singletonKey, schema, testCallback2) - secondHandle.store(entity1) - val expectedAdd = CrdtSingleton.Operation.Update( - singletonKey.toKeyString(), - testMapForKey(singletonKey), - entity1 - ) - verify(testCallback1, times(1)).onUpdate(firstHandle, expectedAdd) - verify(testCallback2, times(1)).onUpdate(secondHandle, expectedAdd) - firstHandle.clear() - - val expectedRemove = CrdtSingleton.Operation.Clear( - singletonKey.toKeyString(), - testMapForKey(singletonKey) - ) - verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove) - verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove) - } + fun singleton_OnHandleUpdate() = runBlocking { + val testCallback1 = mock>() + val testCallback2 = mock>() + val firstHandle = handleManager.singletonHandle(singletonKey, schema, testCallback1) + val secondHandle = handleManager.singletonHandle(singletonKey, schema, testCallback2) + secondHandle.store(entity1) + val expectedAdd = CrdtSingleton.Operation.Update( + singletonKey.toKeyString(), + testMapForKey(singletonKey), + entity1 + ) + verify(testCallback1, times(1)).onUpdate(firstHandle, expectedAdd) + verify(testCallback2, times(1)).onUpdate(secondHandle, expectedAdd) + firstHandle.clear() + + val expectedRemove = CrdtSingleton.Operation.Clear( + singletonKey.toKeyString(), + testMapForKey(singletonKey) + ) + verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove) + verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove) } @Test - fun testSetSyncOnRegister() = runBlockingTest { - handleManagerTest { hm -> - val testCallback = mock>() - val firstHandle = hm.setHandle(setKey, schema, testCallback) - verify(testCallback, times(1)).onSync(firstHandle) - firstHandle.fetchAll() - verify(testCallback, times(1)).onSync(firstHandle) - } + fun set_syncOnRegister() = runBlocking { + val testCallback = mock>() + val firstHandle = handleManager.setHandle(setKey, schema, testCallback) + verify(testCallback, times(1)).onSync(firstHandle) + firstHandle.fetchAll() + verify(testCallback, times(1)).onSync(firstHandle) } @Test - fun testSingletonSyncOnRegister() = runBlockingTest { - handleManagerTest { hm -> - val testCallback = mock>() - val firstHandle = hm.singletonHandle(setKey, schema, testCallback) - verify(testCallback, times(1)).onSync(firstHandle) - firstHandle.fetch() - verify(testCallback, times(1)).onSync(firstHandle) - } + fun testSingletonSyncOnRegister() = runBlocking { + val testCallback = mock>() + val firstHandle = handleManager.singletonHandle(setKey, schema, testCallback) + verify(testCallback, times(1)).onSync(firstHandle) + firstHandle.fetch() + verify(testCallback, times(1)).onSync(firstHandle) } } diff --git a/javatests/arcs/android/storage/handle/AndroidManifest.xml b/javatests/arcs/android/storage/handle/AndroidManifest.xml index 39989a6b2ed..e3951be6d0e 100644 --- a/javatests/arcs/android/storage/handle/AndroidManifest.xml +++ b/javatests/arcs/android/storage/handle/AndroidManifest.xml @@ -16,12 +16,6 @@ - - - - - - diff --git a/javatests/arcs/android/storage/handle/BUILD b/javatests/arcs/android/storage/handle/BUILD index a051ff1f5f6..61c9ed2dc8a 100644 --- a/javatests/arcs/android/storage/handle/BUILD +++ b/javatests/arcs/android/storage/handle/BUILD @@ -8,23 +8,12 @@ licenses(["notice"]) package(default_visibility = ["//visibility:public"]) -kt_android_library( - name = "test_app", - testonly = 1, - srcs = ["TestActivity.kt"], - manifest = ":AndroidManifest.xml", - deps = [ - "//third_party/java/androidx/appcompat", - ], -) - arcs_kt_android_test_suite( name = "handle", srcs = glob(["*Test.kt"]), manifest = "AndroidManifest.xml", package = "arcs.android.storage.handle", deps = [ - ":test_app", "//java/arcs/android/crdt", "//java/arcs/android/storage", "//java/arcs/android/storage/handle", @@ -44,14 +33,12 @@ arcs_kt_android_test_suite( "//java/arcs/sdk/android/storage/service/testutil", "//third_party/android/androidx_test/core", "//third_party/android/androidx_test/ext/junit", - "//third_party/java/androidx/appcompat", "//third_party/java/androidx/work:testing", "//third_party/java/junit:junit-android", "//third_party/java/mockito:mockito-android", "//third_party/java/robolectric", "//third_party/java/truth:truth-android", "//third_party/kotlin/kotlinx_coroutines", - "//third_party/kotlin/kotlinx_coroutines:kotlinx_coroutines_test", "//third_party/kotlin/mockito_kotlin:mockito_kotlin-android", ], ) diff --git a/javatests/arcs/android/storage/handle/TestActivity.kt b/javatests/arcs/android/storage/handle/TestActivity.kt deleted file mode 100644 index c6a27da5cd2..00000000000 --- a/javatests/arcs/android/storage/handle/TestActivity.kt +++ /dev/null @@ -1,5 +0,0 @@ -package arcs.android.storage.handle - -import androidx.appcompat.app.AppCompatActivity - -class TestActivity : AppCompatActivity()