From 5c6310da9d1e372544e816c1dd595ef25e5099bf Mon Sep 17 00:00:00 2001 From: jblebrun Date: Tue, 3 Mar 2020 11:02:34 -0800 Subject: [PATCH] Simplify service test pattern (#4817) * 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 | 108 ++++++------------ .../storage/handle/AndroidManifest.xml | 6 - javatests/arcs/android/storage/handle/BUILD | 14 --- .../android/storage/handle/TestActivity.kt | 5 - 5 files changed, 48 insertions(+), 97 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 72c65cad82f..bfcc4eb2ee7 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 @@ -27,10 +29,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,11 +38,16 @@ 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 val backingKey = RamDiskStorageKey("entities") + private lateinit var handleManager: HandleManager + val entity1 = RawEntity( "entity1", singletons = mapOf( @@ -93,61 +97,35 @@ 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() - } - @Test - fun testCreateSingletonHandle() = 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) + handleManager = AndroidHandleManager( + lifecycle = lifecycle, + context = app, + connectionFactory = DefaultConnectionFactory(app, TestBindingDelegate(app)) + ) } @Test - fun testDereferencingFromSingletonEntity() = handleManagerTest { hm -> - val singleton1Handle = hm.singletonHandle(singletonKey, schema) - val singleton1Handle2 = hm.singletonHandle(singletonKey, schema) + fun singleton_dereferenceEntity() = runBlocking { + val singleton1Handle = handleManager.singletonHandle(singletonKey, schema) + val singleton1Handle2 = handleManager.singletonHandle(singletonKey, schema) singleton1Handle.store(entity1) // Create a second handle for the second entity, so we can store it. - val singleton2Handle = hm.singletonHandle( + val singleton2Handle = handleManager.singletonHandle( ReferenceModeStorageKey(backingKey, RamDiskStorageKey("entity2")), schema ) - val singleton2Handle2 = hm.singletonHandle( + val singleton2Handle2 = handleManager.singletonHandle( ReferenceModeStorageKey(backingKey, RamDiskStorageKey("entity2")), schema ) @@ -171,28 +149,16 @@ class AndroidHandleManagerTest { } @Test - fun testCreateSetHandle() = 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) - } - - @Test - fun testDereferencingFromSetHandleEntity() = handleManagerTest { hm -> - val setHandle = hm.setHandle(setKey, schema) + fun set_dereferenceEntity () = runBlocking { + val setHandle = handleManager.setHandle(setKey, schema) setHandle.store(entity1) setHandle.store(entity2) - val secondHandle = hm.setHandle(setKey, schema) + val secondHandle = handleManager.setHandle(setKey, schema) secondHandle.fetchAll().also { assertThat(it).hasSize(2) }.forEach { entity -> val expectedBestFriend = if (entity.id == "entity1") entity2 else entity1 val actualBestFriend = (entity.singletons["best_friend"] as Reference) - .dereference() + .dereference(coroutineContext) assertThat(actualBestFriend).isEqualTo(expectedBestFriend) } } @@ -200,11 +166,11 @@ class AndroidHandleManagerTest { private fun testMapForKey(key: StorageKey) = VersionMap(key.toKeyString() to 1) @Test - fun testSetHandleOnUpdate() = handleManagerTest { hm -> + fun set_onHandleUpdate() = runBlocking { val testCallback1 = mock>() val testCallback2 = mock>() - val firstHandle = hm.setHandle(setKey, schema, testCallback1) - val secondHandle = hm.setHandle(setKey, schema, testCallback2) + val firstHandle = handleManager.setHandle(setKey, schema, testCallback1) + val secondHandle = handleManager.setHandle(setKey, schema, testCallback2) val expectedAdd = CrdtSet.Operation.Add( setKey.toKeyString(), @@ -226,11 +192,11 @@ class AndroidHandleManagerTest { } @Test - fun testSingletonHandleOnUpdate() = handleManagerTest { hm -> + fun singleton_OnHandleUpdate() = runBlocking { val testCallback1 = mock>() val testCallback2 = mock>() - val firstHandle = hm.singletonHandle(singletonKey, schema, testCallback1) - val secondHandle = hm.singletonHandle(singletonKey, schema, testCallback2) + val firstHandle = handleManager.singletonHandle(singletonKey, schema, testCallback1) + val secondHandle = handleManager.singletonHandle(singletonKey, schema, testCallback2) secondHandle.store(entity1) val expectedAdd = CrdtSingleton.Operation.Update( singletonKey.toKeyString(), @@ -250,18 +216,18 @@ class AndroidHandleManagerTest { } @Test - fun testSetSyncOnRegister() = handleManagerTest { hm -> + fun set_syncOnRegister() = runBlocking { val testCallback = mock>() - val firstHandle = hm.setHandle(setKey, schema, testCallback) + val firstHandle = handleManager.setHandle(setKey, schema, testCallback) verify(testCallback, times(1)).onSync(firstHandle) firstHandle.fetchAll() verify(testCallback, times(1)).onSync(firstHandle) } @Test - fun testSingletonSyncOnRegister() = handleManagerTest { hm -> + fun singleton_syncOnRegister() = runBlocking { val testCallback = mock>() - val firstHandle = hm.singletonHandle(setKey, schema, testCallback) + 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 b77cb3265a7..121e0ce84ab 100644 --- a/javatests/arcs/android/storage/handle/BUILD +++ b/javatests/arcs/android/storage/handle/BUILD @@ -2,22 +2,11 @@ load( "//third_party/java/arcs/build_defs:build_defs.bzl", "arcs_kt_android_test_suite", ) -load("//tools/build_defs/kotlin:rules.bzl", "kt_android_library") 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", size = "medium", @@ -25,7 +14,6 @@ arcs_kt_android_test_suite( manifest = "AndroidManifest.xml", package = "arcs.android.storage.handle", deps = [ - ":test_app", "//java/arcs/android/crdt", "//java/arcs/android/storage", "//java/arcs/android/storage/handle", @@ -45,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()