From 64c1a60a970c1313a6bc00704d816fc28ad61a5c Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Mon, 24 Aug 2026 15:30:32 -0500 Subject: [PATCH] Drain the escaped coroutine error before the next test starts `ConversionViewModelProbeFailureTest.an OutOfMemoryError is not swallowed` deliberately lets a real error escape `viewModelScope.launch`, which has no exception handler by design -- the ViewModel's KDoc says an OOM raised in the probe should reach the thread's handler and take the process down. On the JVM something else takes it. kotlinx-coroutines-test installs a process-wide collector for uncaught coroutine errors, keeps whatever it catches, and hands the backlog to the next `runTest` that starts, which throws UncaughtExceptionsBeforeTest. Every Compose test is a `runTest`: that is how `createComposeRule` runs a composition. So the error lands on an unrelated test in an unrelated file, and the message names neither the test that caused it nor the error's origin. Nothing has hit it yet only because the sole Compose test in the repo happens to run before the ViewModel one. R38 adds six more Compose classes in exactly the two packages that surround it, and the first two of them made the suite fail in two different files on two consecutive runs of identical, green code -- the throw is on a real Dispatchers.IO thread, delivered after the state assertion that ends the test responsible, so which class catches it is a race. Drain it where a Compose rule is built. A @Before cannot: the rule's `runTest` wraps the statement that calls it, so it has already thrown. @BeforeClass cannot either, because Robolectric runs it outside the sandbox classloader, where the collector is a different object. Constructing the rule is early enough, since JUnit builds a fresh test-class instance -- and every @get:Rule field on it -- before evaluating any rule. This is containment, not the cure. The cure is a seam: give the probe hop an injectable dispatcher the way the constructor already does for cleanupDispatcher, so the error has somewhere to land. That is a production change and deserves its own commit. kotlinx-coroutines-test was already on the unit-test classpath through compose-ui-test-junit4; it is declared now because a file imports it. Pinned, like robolectric and the linters: org.jetbrains.kotlinx is not one of the groups the prerelease guard covers, so a float here would be free to take a milestone. Co-Authored-By: Claude Opus 5 (1M context) --- app/build.gradle.kts | 4 ++ .../AppRootRestorationTest.kt | 6 ++- .../EscapedCoroutineErrors.kt | 53 +++++++++++++++++++ gradle/libs.versions.toml | 15 ++++++ 4 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/org/libremediaconverter/EscapedCoroutineErrors.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 66bedbe..6dc6a2a 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -306,6 +306,10 @@ dependencies { // the tests stay green. testImplementation(platform(libs.compose.bom)) testImplementation(libs.compose.ui.test.junit4) + // For `runTest` alone, in EscapedCoroutineErrors.kt. It arrives transitively with the + // rule above anyway; declared because a test file imports it directly, and an import of + // something nobody asked for breaks the day the library that pulled it in stops. + testImplementation(libs.kotlinx.coroutines.test) androidTestImplementation(platform(libs.compose.bom)) androidTestImplementation(libs.androidx.junit) diff --git a/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt b/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt index b0e1c78..07dd1f0 100644 --- a/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt +++ b/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt @@ -8,7 +8,6 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.platform.testTag import androidx.compose.ui.test.assertIsSelected import androidx.compose.ui.test.junit4.StateRestorationTester -import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick @@ -42,8 +41,11 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class AppRootRestorationTest { + // Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors]. Every Compose test + // class in this source set starts there, whether or not it is the one that happens to be + // running when another test's escaped coroutine error is delivered. @get:Rule - val composeRule = createComposeRule() + val composeRule = createDrainedComposeRule() private val restoration = StateRestorationTester(composeRule) diff --git a/app/src/test/java/org/libremediaconverter/EscapedCoroutineErrors.kt b/app/src/test/java/org/libremediaconverter/EscapedCoroutineErrors.kt new file mode 100644 index 0000000..e27782f --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/EscapedCoroutineErrors.kt @@ -0,0 +1,53 @@ +package org.libremediaconverter + +import androidx.compose.ui.test.junit4.v2.createComposeRule +import kotlinx.coroutines.test.runTest + +/** + * Clears coroutine errors this module's tests deliberately let escape, so they land on the test + * that caused them instead of on the next one to start. + * + * **Every Compose test class in `src/test` has to start here.** `createComposeRule` runs the + * composition inside `runTest`, and `runTest` opens by throwing `UncaughtExceptionsBeforeTest` for + * anything already sitting in kotlinx-coroutines-test's collector -- a process-wide + * `CoroutineExceptionHandler` it installs once and never removes. + * + * There is one deposit into that collector here, and it is not a mistake: + * `ConversionViewModelProbeFailureTest.an OutOfMemoryError is not swallowed` proves an OOM raised + * inside the probe is rethrown rather than reported as an unreadable file. `onInputPicked` runs it + * in `viewModelScope.launch`, which has no exception handler by design -- the ViewModel's own KDoc + * says a real OOM should reach the thread's handler and take the process down. On the JVM the + * collector takes it instead, holds it, and hands it to whichever `runTest` starts next. + * + * It surfaced as two *different* Compose test classes failing on two consecutive runs of the same, + * green, code, with a message naming neither the test nor the error's origin. Which class catches + * it moves because the throw happens on a real `Dispatchers.IO` thread, after the state assertion + * that ends the test that caused it -- so it can be delivered long after that class is done. + * + * A `@Before` method cannot do this: the compose rule's `runTest` wraps the statement that calls + * `@Before`, so it has already thrown. `@BeforeClass` cannot either -- Robolectric runs it outside + * the sandbox classloader, where the collector is a different object. Draining while the rule is + * being *constructed* is early enough, because JUnit builds a fresh test-class instance, and with + * it every `@get:Rule` field, before evaluating any rule. + * + * The real fix is a seam: give the probe hop an injectable dispatcher the way + * `ConversionViewModel`'s constructor already does for `cleanupDispatcher`, and the error would + * have somewhere to land. That is a production change, so it belongs in its own commit. + */ +fun drainEscapedCoroutineErrors() { + // Entering a test scope is what flushes the collector; the flush is reported as this + // throwing, and there is nothing to assert about an error another test already asserted on. + runCatching { runTest {} } +} + +/** + * [createComposeRule], with [drainEscapedCoroutineErrors] run first. Use this rather than + * `createComposeRule` directly in `src/test`. + * + * It also keeps the one mixed import in one place: the rule comes from the **v2** package + * (`androidx.compose.ui.test.junit4.v2`) while `StateRestorationTester`, which takes it, does not. + */ +fun createDrainedComposeRule() = run { + drainEscapedCoroutineErrors() + createComposeRule() +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7b96107..8441472 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -80,6 +80,17 @@ jacoco = "0.8.15" # 4.16.1 is the newest RELEASED version; the 4.17 line is beta-only at the time of writing. robolectric = "4.16.1" +# kotlinx-coroutines-test. Already on the unit-test classpath transitively, through +# compose-ui-test-junit4 -- declared here because a source file now imports it, and a direct +# import of a transitive is a dependency nobody chose. +# +# PINNED, for the same reason as robolectric above: org.jetbrains.kotlinx is not one of the +# groups in the prerelease guard's `floatedGroupPrefixes`, so a "1.+" here would be free to +# resolve to a milestone build. This value is what the Compose BOM already resolves it to, so +# stating it changes nothing in the graph today; if the BOM moves ahead, Gradle takes the +# higher version and this stays a floor rather than a conflict. +coroutinesTest = "1.9.0" + [libraries] androidx-core-ktx = { group = "androidx.core", name = "core-ktx", version.ref = "coreKtx" } androidx-activity-compose = { group = "androidx.activity", name = "activity-compose", version.ref = "activityCompose" } @@ -134,6 +145,10 @@ androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-co # a TDD loop anyone here can execute. robolectric = { group = "org.robolectric", name = "robolectric", version.ref = "robolectric" } +# Only for its `runTest`, and only to drain the collector kotlinx-coroutines-test installs +# process-wide. See EscapedCoroutineErrors.kt in the JVM test source set. +kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutinesTest" } + [plugins] # com.android.application and org.jetbrains.kotlin.plugin.compose are deliberately absent. # They come from the root buildscript classpath (see build.gradle.kts) so that a newer KGP