From 4aba3bbd2e46b609bea4c6bdfa0066981e580acc Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Mon, 24 Aug 2026 23:10:04 -0500 Subject: [PATCH] Stop swallowing coroutine errors nobody asserted on `drainEscapedCoroutineErrors()` cleared the collector at rule-construction time with `runCatching { runTest {} }`, and discarding what it found was the whole mechanism: it could not tell the one known deposit from an escaped error nobody had asserted on. That traded a loud, misleading failure for a silent one, which was acceptable only while exactly one depositor existed and the seam to remove it did not. The seam exists now, so the depositor is gone: the OOM is consumed by the test that raises it. Every Compose class takes the v2 `createComposeRule()` directly, and a future escaped error fails a test again instead of disappearing. The two findings the drain's KDoc carried that outlive it: the v2 rule and the non-v2 `StateRestorationTester` do interoperate -- the note now sits at the two declarations that pair them -- and a drain could never have been a `@Before` (the rule's `runTest` wraps it) or a `@BeforeClass` (Robolectric runs that outside the sandbox classloader, where the collector is a different object). Full JVM suite run twice in a row with the drain deleted: 373 tests, 0 failures both times. Closes #66 Co-Authored-By: Claude Opus 5 (1M context) --- .../AppRootRestorationTest.kt | 9 ++-- .../EscapedCoroutineErrors.kt | 53 ------------------- .../convert/AdvancedPanelSavedStateTest.kt | 5 +- .../convert/AdvancedPickerTest.kt | 8 +-- .../convert/ConverterLeafTagsTest.kt | 4 +- .../convert/ConverterPickerSelectionTest.kt | 4 +- .../convert/ConverterScreenContentTest.kt | 5 +- .../convert/ConverterStateAffordancesTest.kt | 5 +- .../convert/FileCardTest.kt | 4 +- .../join/JoinLeafTagsTest.kt | 4 +- .../join/JoinScreenContentTest.kt | 5 +- .../join/JoinStateAffordancesTest.kt | 5 +- 12 files changed, 28 insertions(+), 83 deletions(-) delete mode 100644 app/src/test/java/org/libremediaconverter/EscapedCoroutineErrors.kt diff --git a/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt b/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt index 07dd1f0..8219b58 100644 --- a/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt +++ b/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt @@ -8,6 +8,7 @@ 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 @@ -41,11 +42,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. + // The rule is the **v2** one (`androidx.compose.ui.test.junit4.v2`) while + // [StateRestorationTester], which takes it below, is not. The mismatched imports are + // deliberate: the v2 package has no tester of its own and the two do interoperate. @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() 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 deleted file mode 100644 index e27782f..0000000 --- a/app/src/test/java/org/libremediaconverter/EscapedCoroutineErrors.kt +++ /dev/null @@ -1,53 +0,0 @@ -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/app/src/test/java/org/libremediaconverter/convert/AdvancedPanelSavedStateTest.kt b/app/src/test/java/org/libremediaconverter/convert/AdvancedPanelSavedStateTest.kt index 4961381..c48e7b9 100644 --- a/app/src/test/java/org/libremediaconverter/convert/AdvancedPanelSavedStateTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/AdvancedPanelSavedStateTest.kt @@ -7,6 +7,7 @@ import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.MutableState import androidx.compose.runtime.saveable.LocalSaveableStateRegistry import androidx.compose.runtime.saveable.SaveableStateRegistry +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick import androidx.media3.common.util.UnstableApi @@ -15,7 +16,6 @@ import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.AudioCodec import org.libremediaconverter.model.Container import org.libremediaconverter.model.OutputSpec @@ -57,9 +57,8 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class AdvancedPanelSavedStateTest { - // Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors]. @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() /** * `canBeSaved = { true }` deliberately. diff --git a/app/src/test/java/org/libremediaconverter/convert/AdvancedPickerTest.kt b/app/src/test/java/org/libremediaconverter/convert/AdvancedPickerTest.kt index 342b54e..f7c7e94 100644 --- a/app/src/test/java/org/libremediaconverter/convert/AdvancedPickerTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/AdvancedPickerTest.kt @@ -6,6 +6,7 @@ import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasTestTag import androidx.compose.ui.test.hasText import androidx.compose.ui.test.junit4.StateRestorationTester +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onAllNodesWithTag import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText @@ -16,7 +17,6 @@ import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.AudioCodec import org.libremediaconverter.model.Container import org.libremediaconverter.model.ContainerCapabilities @@ -61,9 +61,11 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class AdvancedPickerTest { - // Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors]. + // The rule is the **v2** one (`androidx.compose.ui.test.junit4.v2`) while + // [StateRestorationTester], which takes it below, is not. The mismatched imports are + // deliberate: the v2 package has no tester of its own and the two do interoperate. @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() private val restoration = StateRestorationTester(composeRule) diff --git a/app/src/test/java/org/libremediaconverter/convert/ConverterLeafTagsTest.kt b/app/src/test/java/org/libremediaconverter/convert/ConverterLeafTagsTest.kt index 0a64b24..8093458 100644 --- a/app/src/test/java/org/libremediaconverter/convert/ConverterLeafTagsTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/ConverterLeafTagsTest.kt @@ -2,6 +2,7 @@ package org.libremediaconverter.convert import android.net.Uri import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onAllNodesWithTag import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick @@ -9,7 +10,6 @@ import androidx.media3.common.util.UnstableApi import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.AudioCodec import org.libremediaconverter.model.Container import org.libremediaconverter.model.EnginePreference @@ -47,7 +47,7 @@ import org.robolectric.RobolectricTestRunner class ConverterLeafTagsTest { @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() private fun assertResolvesToOneNode(tag: String) { composeRule.onAllNodesWithTag(tag).assertCountEquals(1) diff --git a/app/src/test/java/org/libremediaconverter/convert/ConverterPickerSelectionTest.kt b/app/src/test/java/org/libremediaconverter/convert/ConverterPickerSelectionTest.kt index 3fdbbb6..da1992f 100644 --- a/app/src/test/java/org/libremediaconverter/convert/ConverterPickerSelectionTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/ConverterPickerSelectionTest.kt @@ -6,6 +6,7 @@ import androidx.compose.ui.test.assertIsSelected import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasTestTag import androidx.compose.ui.test.hasText +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.media3.common.util.UnstableApi @@ -13,7 +14,6 @@ import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.EnginePreference import org.libremediaconverter.model.OutputFormat import org.libremediaconverter.model.QualityTier @@ -48,7 +48,7 @@ import org.robolectric.RobolectricTestRunner class ConverterPickerSelectionTest { @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() /** * The chip carrying [label] inside the row tagged [rowTag]. diff --git a/app/src/test/java/org/libremediaconverter/convert/ConverterScreenContentTest.kt b/app/src/test/java/org/libremediaconverter/convert/ConverterScreenContentTest.kt index 3f377ba..6964ab0 100644 --- a/app/src/test/java/org/libremediaconverter/convert/ConverterScreenContentTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/ConverterScreenContentTest.kt @@ -1,6 +1,7 @@ package org.libremediaconverter.convert import android.net.Uri +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollTo @@ -9,7 +10,6 @@ import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.Validation import org.libremediaconverter.ui.TestTags import org.robolectric.RobolectricTestRunner @@ -39,9 +39,8 @@ import java.io.File @RunWith(RobolectricTestRunner::class) class ConverterScreenContentTest { - // Not `createComposeRule()` directly: see [org.libremediaconverter.drainEscapedCoroutineErrors]. @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() /** What the screen asked to save, in the order it asked. Empty until Save is tapped. */ private val savedAs = mutableListOf() diff --git a/app/src/test/java/org/libremediaconverter/convert/ConverterStateAffordancesTest.kt b/app/src/test/java/org/libremediaconverter/convert/ConverterStateAffordancesTest.kt index a7f0447..5d459f8 100644 --- a/app/src/test/java/org/libremediaconverter/convert/ConverterStateAffordancesTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/ConverterStateAffordancesTest.kt @@ -6,6 +6,7 @@ import androidx.compose.ui.test.assertIsEnabled import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.assertRangeInfoEquals import androidx.compose.ui.test.assertTextEquals +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 @@ -15,7 +16,6 @@ import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.AudioCodec import org.libremediaconverter.model.Container import org.libremediaconverter.model.OutputSpec @@ -72,9 +72,8 @@ import java.io.File @RunWith(RobolectricTestRunner::class) class ConverterStateAffordancesTest { - // Not `createComposeRule()` directly: see [org.libremediaconverter.drainEscapedCoroutineErrors]. @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() /** * Every callback the screen fired, in order, tagged with the value it carried. diff --git a/app/src/test/java/org/libremediaconverter/convert/FileCardTest.kt b/app/src/test/java/org/libremediaconverter/convert/FileCardTest.kt index 703f932..a6e8112 100644 --- a/app/src/test/java/org/libremediaconverter/convert/FileCardTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/FileCardTest.kt @@ -3,13 +3,13 @@ package org.libremediaconverter.convert import android.net.Uri import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.assertTextEquals +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onChildren import androidx.compose.ui.test.onNodeWithTag import androidx.media3.common.util.UnstableApi import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.AudioCodec import org.libremediaconverter.model.Container import org.libremediaconverter.model.InputKind @@ -56,7 +56,7 @@ import org.robolectric.RobolectricTestRunner class FileCardTest { @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() @Test fun `a file no provider could measure says so in words rather than showing a zero`() { diff --git a/app/src/test/java/org/libremediaconverter/join/JoinLeafTagsTest.kt b/app/src/test/java/org/libremediaconverter/join/JoinLeafTagsTest.kt index 6316057..dec1b8f 100644 --- a/app/src/test/java/org/libremediaconverter/join/JoinLeafTagsTest.kt +++ b/app/src/test/java/org/libremediaconverter/join/JoinLeafTagsTest.kt @@ -2,13 +2,13 @@ package org.libremediaconverter.join import android.net.Uri import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onAllNodesWithTag import androidx.media3.common.util.UnstableApi import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.libremediaconverter.convert.InputFile -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.ui.TestTags import org.robolectric.RobolectricTestRunner @@ -32,7 +32,7 @@ import org.robolectric.RobolectricTestRunner class JoinLeafTagsTest { @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() private fun input(displayName: String) = InputFile( uri = Uri.parse("content://test/$displayName"), diff --git a/app/src/test/java/org/libremediaconverter/join/JoinScreenContentTest.kt b/app/src/test/java/org/libremediaconverter/join/JoinScreenContentTest.kt index 478e814..a3f40b6 100644 --- a/app/src/test/java/org/libremediaconverter/join/JoinScreenContentTest.kt +++ b/app/src/test/java/org/libremediaconverter/join/JoinScreenContentTest.kt @@ -1,5 +1,6 @@ package org.libremediaconverter.join +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollTo @@ -8,7 +9,6 @@ import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.ConcatStrategy import org.libremediaconverter.ui.TestTags import org.robolectric.RobolectricTestRunner @@ -35,9 +35,8 @@ import java.io.File @RunWith(RobolectricTestRunner::class) class JoinScreenContentTest { - // Not `createComposeRule()` directly: see [org.libremediaconverter.drainEscapedCoroutineErrors]. @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() /** What the screen asked to save, in the order it asked. Empty until Save is tapped. */ private val savedAs = mutableListOf() diff --git a/app/src/test/java/org/libremediaconverter/join/JoinStateAffordancesTest.kt b/app/src/test/java/org/libremediaconverter/join/JoinStateAffordancesTest.kt index 21ababd..f446d2d 100644 --- a/app/src/test/java/org/libremediaconverter/join/JoinStateAffordancesTest.kt +++ b/app/src/test/java/org/libremediaconverter/join/JoinStateAffordancesTest.kt @@ -7,6 +7,7 @@ import androidx.compose.ui.semantics.getOrNull import androidx.compose.ui.test.SemanticsMatcher import androidx.compose.ui.test.assertRangeInfoEquals import androidx.compose.ui.test.assertTextEquals +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 @@ -17,7 +18,6 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.libremediaconverter.convert.InputFile -import org.libremediaconverter.createDrainedComposeRule import org.libremediaconverter.model.ConcatStrategy import org.libremediaconverter.ui.TestTags import org.robolectric.RobolectricTestRunner @@ -60,9 +60,8 @@ import java.io.File @RunWith(RobolectricTestRunner::class) class JoinStateAffordancesTest { - // Not `createComposeRule()` directly: see [org.libremediaconverter.drainEscapedCoroutineErrors]. @get:Rule - val composeRule = createDrainedComposeRule() + val composeRule = createComposeRule() /** Which callback the screen invoked, in order, with what it passed. Empty until one fires. */ private val events = mutableListOf()