Tweak run stopping

ooni · Nov 12, 2024 · 16ac9a6 · 16ac9a6
1 parent 88c02fe
commit 16ac9a6
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 24 deletions.
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/engine/Engine.kt b/composeApp/src/commonMain/kotlin/org/ooni/engine/Engine.kt
@@ -2,7 +2,6 @@ package org.ooni.engine
 
 import androidx.annotation.VisibleForTesting
 import co.touchlab.kermit.Logger
-import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.async
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.channelFlow
@@ -37,14 +36,14 @@ class Engine(
     private val isBatteryCharging: suspend () -> Boolean,
     private val platformInfo: PlatformInfo,
     private val getEnginePreferences: suspend () -> EnginePreferences,
+    private val observeCancelTestRun: () -> Flow<Unit>,
     private val backgroundContext: CoroutineContext,
 ) {
     fun startTask(
         name: String,
         inputs: List<String>?,
         taskOrigin: TaskOrigin,
         descriptorId: InstalledTestDescriptorModel.Id?,
-        observeCancelTestRun: Flow<Unit>,
     ): Flow<TaskEvent> =
         channelFlow {
             val preferences = getEnginePreferences()
@@ -56,7 +55,7 @@ class Engine(
                 task = bridge.startTask(settingsSerialized)
 
                 val cancelJob = async {
-                    observeCancelTestRun
+                    observeCancelTestRun()
                         .take(1)
                         .collect {
                             task.interrupt()
@@ -68,15 +67,17 @@ class Engine(
                     val taskEventResult = json.decodeFromString<TaskEventResult>(eventJson)
                     taskEventMapper(taskEventResult)?.let { send(it) }
                 }
-                cancelJob.cancel()
-            } catch (e: CancellationException) {
-                Logger.d("Test cancelled")
-                throw e
+
+                if (cancelJob.isActive) {
+                    cancelJob.cancel()
+                }
             } catch (e: Exception) {
                 Logger.d("Error while running task", e)
                 throw MkException(e)
             } finally {
-                task?.interrupt()
+                if (task?.isDone() == false) {
+                    task.interrupt()
+                }
             }
         }.flowOn(backgroundContext)
 

diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/di/Dependencies.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/di/Dependencies.kt
@@ -183,6 +183,7 @@ class Dependencies(
             isBatteryCharging = isBatteryCharging,
             platformInfo = platformInfo,
             getEnginePreferences = getEnginePreferences::invoke,
+            observeCancelTestRun = testStateManager::observeCancels,
             backgroundContext = backgroundContext,
         )
     }
@@ -355,7 +356,6 @@ class Dependencies(
             deleteFiles = deleteFiles,
             json = json,
             spec = spec,
-            observeCancelTestRun = testStateManager.observeCancels(),
         )
 
     // Background

diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/RunDescriptors.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/RunDescriptors.kt
@@ -65,16 +65,6 @@ class RunDescriptors(
         spec: RunSpecification,
     ) {
         coroutineScope {
-            val runJob = async {
-                // Actually running the descriptors
-                descriptors.forEachIndexed { index, descriptor ->
-                    // check if cancel has been requested before running descriptor
-                    if (getCurrentTestRunState.first() is TestRunState.Stopping) {
-                        return@forEachIndexed
-                    }
-                    runDescriptor(descriptor, index, spec.taskOrigin, spec.isRerun)
-                }
-            }
             // Observe if a cancel request has been made
             val cancelJob = async {
                 observeCancelTestRun
@@ -84,7 +74,11 @@ class RunDescriptors(
                     }
             }
 
-            runJob.await()
+            // Actually running the descriptors
+            descriptors.forEachIndexed { index, descriptor ->
+                if (isRunStopped()) return@forEachIndexed
+                runDescriptor(descriptor, index, spec.taskOrigin, spec.isRerun)
+            }
 
             if (cancelJob.isActive) {
                 cancelJob.cancel()
@@ -142,6 +136,7 @@ class RunDescriptors(
         val resultId = storeResult(result)
 
         descriptor.allTests.forEachIndexed { testIndex, netTest ->
+            if (isRunStopped()) return@forEachIndexed
             runNetTest(
                 RunNetTest.Specification(
                     descriptor = descriptor,
@@ -158,4 +153,6 @@ class RunDescriptors(
 
         markResultAsDone(resultId)
     }
+
+    private suspend fun isRunStopped() = getCurrentTestRunState.first() is TestRunState.Stopping
 }
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/RunNetTest.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/RunNetTest.kt
@@ -21,7 +21,7 @@ import org.ooni.probe.data.models.UrlModel
 import org.ooni.probe.shared.toLocalDateTime
 
 class RunNetTest(
-    private val startTest: (String, List<String>?, TaskOrigin, InstalledTestDescriptorModel.Id?, Flow<Unit>) -> Flow<TaskEvent>,
+    private val startTest: (String, List<String>?, TaskOrigin, InstalledTestDescriptorModel.Id?) -> Flow<TaskEvent>,
     private val getOrCreateUrl: suspend (String) -> UrlModel,
     private val storeMeasurement: suspend (MeasurementModel) -> MeasurementModel.Id,
     private val storeNetwork: suspend (NetworkModel) -> NetworkModel.Id,
@@ -31,7 +31,6 @@ class RunNetTest(
     private val deleteFiles: DeleteFiles,
     private val json: Json,
     private val spec: Specification,
-    private val observeCancelTestRun: Flow<Unit>,
 ) {
     data class Specification(
         val descriptor: Descriptor,
@@ -69,7 +68,6 @@ class RunNetTest(
             spec.netTest.inputs,
             spec.taskOrigin,
             installedDescriptorId,
-            observeCancelTestRun,
         )
             .collect(::onEvent)
     }

diff --git a/composeApp/src/commonTest/kotlin/org/ooni/engine/EngineTest.kt b/composeApp/src/commonTest/kotlin/org/ooni/engine/EngineTest.kt
@@ -34,7 +34,6 @@ class EngineTest {
                 inputs = listOf("https://ooni.org"),
                 taskOrigin = TaskOrigin.OoniRun,
                 descriptorId = null,
-                observeCancelTestRun = emptyFlow(),
             ).toList()
 
             assertEquals(1, events.size)
@@ -87,6 +86,7 @@ class EngineTest {
                     maxRuntime = null,
                 )
             },
+            observeCancelTestRun = { emptyFlow() },
             backgroundContext = Dispatchers.Unconfined,
         )
 }