Fix UnifiedPushService tests (#1420)

Author: rfc2822

app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt

@@ -8,6 +8,7 @@ import android.content.Context
 import android.content.Intent
 import android.os.IBinder
 import androidx.test.rule.ServiceTestRule
+import at.bitfire.davdroid.di.MainDispatcher
 import dagger.hilt.android.qualifiers.ApplicationContext
 import dagger.hilt.android.testing.BindValue
 import dagger.hilt.android.testing.HiltAndroidRule
@@ -18,6 +19,10 @@ import io.mockk.every
 import io.mockk.impl.annotations.RelaxedMockK
 import io.mockk.junit4.MockKRule
 import io.mockk.mockk
+import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.advanceUntilIdle
+import kotlinx.coroutines.test.runTest
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -26,6 +31,7 @@ import org.unifiedpush.android.connector.PushService
 import org.unifiedpush.android.connector.data.PushEndpoint
 import javax.inject.Inject
 
+@OptIn(ExperimentalCoroutinesApi::class)
 @HiltAndroidTest
 class UnifiedPushServiceTest {
 
@@ -42,6 +48,10 @@ class UnifiedPushServiceTest {
     @ApplicationContext
     lateinit var context: Context
 
+    @Inject
+    @MainDispatcher
+    lateinit var mainDispatcher: CoroutineDispatcher
+
     @RelaxedMockK
     @BindValue
     lateinit var pushRegistrationManager: PushRegistrationManager
@@ -60,32 +70,35 @@ class UnifiedPushServiceTest {
 
 
     @Test
-    fun testOnNewEndpoint() {
+    fun testOnNewEndpoint() = runTest(mainDispatcher) {
         val endpoint = mockk<PushEndpoint> {
             every { url } returns "https://example.com/12"
         }
         unifiedPushService.onNewEndpoint(endpoint, "12")
 
+        advanceUntilIdle()
         coVerify {
             pushRegistrationManager.processSubscription(12, endpoint)
         }
         confirmVerified(pushRegistrationManager)
     }
 
     @Test
-    fun testOnRegistrationFailed() {
+    fun testOnRegistrationFailed() = runTest(mainDispatcher) {
         unifiedPushService.onRegistrationFailed(FailedReason.INTERNAL_ERROR, "34")
 
+        advanceUntilIdle()
         coVerify {
             pushRegistrationManager.removeSubscription(34)
         }
         confirmVerified(pushRegistrationManager)
     }
 
     @Test
-    fun testOnUnregistered() {
+    fun testOnUnregistered() = runTest(mainDispatcher) {
         unifiedPushService.onRegistrationFailed(FailedReason.INTERNAL_ERROR, "45")
 
+        advanceUntilIdle()
         coVerify {
             pushRegistrationManager.removeSubscription(45)
         }

app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt

@@ -4,10 +4,10 @@
 
 package at.bitfire.davdroid.push
 
+import at.bitfire.davdroid.di.ApplicationScope
 import dagger.Lazy
 import dagger.hilt.android.AndroidEntryPoint
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.launch
 import org.unifiedpush.android.connector.FailedReason
 import org.unifiedpush.android.connector.PushService
@@ -25,6 +25,13 @@ import javax.inject.Inject
 @AndroidEntryPoint
 class UnifiedPushService : PushService() {
 
+    /* Scope to run the requests asynchronously. UnifiedPush binds the service,
+    * sends the message and unbinds one second later. Our operations may take longer,
+    * so the scope should not be bound to the service lifecycle. */
+    @Inject
+    @ApplicationScope
+    lateinit var applicationScope: CoroutineScope
+
     @Inject
     lateinit var logger: Logger
 
@@ -34,18 +41,13 @@ class UnifiedPushService : PushService() {
     @Inject
     lateinit var pushRegistrationManager: Lazy<PushRegistrationManager>
 
-    /* Scope to run the requests asynchronously. UnifiedPush binds the service,
-    * sends the message and unbinds one second later. Our operations may take longer,
-    * so the scope should not be bound to the service lifecycle. */
-    val serviceScope = CoroutineScope(SupervisorJob())
-
 
     override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) {
         val serviceId = instance.toLongOrNull() ?: return
         logger.warning("Got UnifiedPush endpoint for service $serviceId: ${endpoint.url}")
 
         // register new endpoint at CalDAV/CardDAV servers
-        serviceScope.launch {
+        applicationScope.launch {
             pushRegistrationManager.get().processSubscription(serviceId, endpoint)
         }
     }
@@ -55,7 +57,7 @@ class UnifiedPushService : PushService() {
         logger.warning("UnifiedPush registration failed for service $serviceId: $reason")
 
         // unregister subscriptions
-        serviceScope.launch {
+        applicationScope.launch {
             pushRegistrationManager.get().removeSubscription(serviceId)
         }
     }
@@ -64,13 +66,13 @@ class UnifiedPushService : PushService() {
         val serviceId = instance.toLongOrNull() ?: return
         logger.warning("UnifiedPush unregistered for service $serviceId")
 
-        serviceScope.launch {
+        applicationScope.launch {
             pushRegistrationManager.get().removeSubscription(serviceId)
         }
     }
 
     override fun onMessage(message: PushMessage, instance: String) {
-        serviceScope.launch {
+        applicationScope.launch {
             pushMessageHandler.get().processMessage(message, instance)
         }
     }