From 944fa45d7290b57f39bd4b4b088d33000095a163 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 use `runBlocking`. --- javatests/arcs/android/host/BUILD | 12 +- .../handle/AndroidHandleManagerTest.kt | 214 ++++++++---------- .../storage/handle/AndroidManifest.xml | 6 - javatests/arcs/android/storage/handle/BUILD | 13 -- .../android/storage/handle/TestActivity.kt | 5 - 5 files changed, 107 insertions(+), 143 deletions(-) delete mode 100644 javatests/arcs/android/storage/handle/TestActivity.kt diff --git a/javatests/arcs/android/host/BUILD b/javatests/arcs/android/host/BUILD index 055bccd4533..ba4ab0d17f9 100644 --- a/javatests/arcs/android/host/BUILD +++ b/javatests/arcs/android/host/BUILD @@ -9,6 +9,16 @@ 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 = "host", srcs = glob(["*Test.kt"]), @@ -17,6 +27,7 @@ arcs_kt_android_test_suite( deps = [ ":schemas", ":services", + ":test_app", "//java/arcs/android/crdt", "//java/arcs/android/host", "//java/arcs/android/sdk/host", @@ -42,7 +53,6 @@ arcs_kt_android_test_suite( "//java/arcs/sdk/android/storage", "//java/arcs/sdk/android/storage/service", "//java/arcs/sdk/android/storage/service/testutil", - "//javatests/arcs/android/storage/handle:test_app", "//javatests/arcs/core/allocator:allocator-test-util", "//third_party/android/androidx_test/core", "//third_party/android/androidx_test/ext/junit", diff --git a/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt b/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt index 2aa9512e07c..243404d24e6 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,10 +28,7 @@ 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.launch import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.test.TestCoroutineScope -import kotlinx.coroutines.test.runBlockingTest import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -39,27 +38,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 +92,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()