Simplify service test pattern

* Make a simple test lifecycle registry, instead of creating empty
testactivity

* Remove use of `runBlockingTest`: according to
Kotlin/kotlinx.coroutines#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`.

Author: jblebrun

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

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<Unit> {
+        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<SetCallbacks<RawEntity>>()
-            val testCallback2 = mock<SetCallbacks<RawEntity>>()
-            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<Unit> {
+        val testCallback1 = mock<SetCallbacks<RawEntity>>()
+        val testCallback2 = mock<SetCallbacks<RawEntity>>()
+        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<SingletonCallbacks<RawEntity>>()
-            val testCallback2 = mock<SingletonCallbacks<RawEntity>>()
-            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<RawEntity>(
-                singletonKey.toKeyString(),
-                testMapForKey(singletonKey)
-            )
-            verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove)
-            verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove)
-        }
+    fun singleton_OnHandleUpdate() = runBlocking<Unit> {
+        val testCallback1 = mock<SingletonCallbacks<RawEntity>>()
+        val testCallback2 = mock<SingletonCallbacks<RawEntity>>()
+        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<RawEntity>(
+            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<SetCallbacks<RawEntity>>()
-            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<Unit> {
+        val testCallback = mock<SetCallbacks<RawEntity>>()
+        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<SingletonCallbacks<RawEntity>>()
-            val firstHandle = hm.singletonHandle(setKey, schema, testCallback)
-            verify(testCallback, times(1)).onSync(firstHandle)
-            firstHandle.fetch()
-            verify(testCallback, times(1)).onSync(firstHandle)
-        }
+    fun testSingletonSyncOnRegister() = runBlocking<Unit> {
+        val testCallback = mock<SingletonCallbacks<RawEntity>>()
+        val firstHandle = handleManager.singletonHandle(setKey, schema, testCallback)
+        verify(testCallback, times(1)).onSync(firstHandle)
+        firstHandle.fetch()
+        verify(testCallback, times(1)).onSync(firstHandle)
     }
 }

javatests/arcs/android/storage/handle/AndroidManifest.xml

@@ -16,12 +16,6 @@
 
     <application>
         <service android:name="arcs.sdk.android.service.StorageService" android:exported="false"/>
-        <activity android:name="arcs.android.storage.handle.TestActivity" >
-        <intent-filter>
-          <action android:name="android.intent.action.MAIN"/>
-          <category android:name="android.intent.category.LAUNCHER"/>
-        </intent-filter>
-      </activity>
     </application>
 
 </manifest>

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