JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

Give the probe hop an injectable dispatcher, so an escaped coroutine error fails the test that caused it #66

New Issue
Closed
opened 2026-08-24 20:46:13 +00:00 by JMR-dev · 1 comment
JMR-dev commented 2026-08-24 20:46:13 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Found while implementing #57. The workaround landed there; this is the fix it defers to, plus a gap the workaround introduces.

The escaped error

ConversionViewModelProbeFailureTest.an OutOfMemoryError is not swallowed proves an OOM raised
inside the probe is rethrown rather than reported as an unreadable file. ConversionViewModel.onInputPicked
runs the probe in viewModelScope.launch { withContext(Dispatchers.IO) { ... } } with no exception
handler, by design — the ViewModel's KDoc says a real OOM should reach the thread's handler and take
the process down.

On the JVM there is no such handler. kotlinx-coroutines-test installs a process-wide
CoroutineExceptionHandler once and never removes it, so the error is collected and handed to
whichever runTest starts next. Every Compose rule is a runTest.

It surfaced during #57 as two different test classes failing on two consecutive runs of identical,
green code, with a message naming neither the test nor the error's origin. The throw happens on a
real Dispatchers.IO thread after the state assertion that ends the test causing it, so delivery
can land arbitrarily late.

What #57 did, and what it costs

createDrainedComposeRule() in app/src/test/java/org/libremediaconverter/EscapedCoroutineErrors.kt
drains the collector at rule-construction time. @Before cannot (the rule's runTest wraps it) and
@BeforeClass cannot (Robolectric runs it outside the sandbox classloader). It works, and every
Compose test in src/test now depends on it.

The gap: runCatching { runTest {} } discards whatever it finds. It cannot distinguish the one
known deposit from a genuinely unexpected escaped error, so a future coroutine failure that nobody
asserted on becomes silent rather than failing a random later test. That trades a loud, misleading
symptom for a quiet one. Acceptable as a stopgap because there is exactly one known depositor;
not acceptable as the permanent answer, which is why this ticket exists.

The fix

Give the probe hop an injectable dispatcher, exactly as ConversionViewModel's constructor already
does for cleanupDispatcher:

class ConversionViewModel @JvmOverloads constructor(
    app: Application,
    cleanupDispatcher: CoroutineDispatcher = Dispatchers.IO,
    // add: probeDispatcher, so a test can put the hop on its own scheduler
)

With the hop on a test dispatcher, the error has somewhere to land: it fails the test that caused it,
by name, and nothing leaks into the process-wide collector. Then createDrainedComposeRule() can lose
the drain and become a thin alias — or go away entirely.

Done means

  • The dispatcher seam exists and ConversionViewModelProbeFailureTest asserts the OOM at the call
    rather than only through the resulting state.
  • drainEscapedCoroutineErrors() is removed, or reduced to something that no longer swallows
    unknown errors.
  • The mutation: revert the seam, run the full JVM suite twice in a row, and confirm the flake
    returns — a fix for a heisenbug that cannot be shown to reappear has not been shown to work.
  • Gate: ./gradlew :app:assembleDebug :app:testDebugUnitTest :app:compileDebugAndroidTestKotlin :app:ktlintCheck :app:detekt :app:lintDebug --continue
_Found while implementing #57. The workaround landed there; this is the fix it defers to, plus a gap the workaround introduces._ ### The escaped error `ConversionViewModelProbeFailureTest.an OutOfMemoryError is not swallowed` proves an OOM raised inside the probe is rethrown rather than reported as an unreadable file. `ConversionViewModel.onInputPicked` runs the probe in `viewModelScope.launch { withContext(Dispatchers.IO) { ... } }` with **no exception handler, by design** — the ViewModel's KDoc says a real OOM should reach the thread's handler and take the process down. On the JVM there is no such handler. **kotlinx-coroutines-test installs a process-wide `CoroutineExceptionHandler` once and never removes it**, so the error is collected and handed to whichever `runTest` starts *next*. Every Compose rule is a `runTest`. It surfaced during #57 as **two different test classes failing on two consecutive runs of identical, green code**, with a message naming neither the test nor the error's origin. The throw happens on a real `Dispatchers.IO` thread *after* the state assertion that ends the test causing it, so delivery can land arbitrarily late. ### What #57 did, and what it costs `createDrainedComposeRule()` in `app/src/test/java/org/libremediaconverter/EscapedCoroutineErrors.kt` drains the collector at rule-construction time. `@Before` cannot (the rule's `runTest` wraps it) and `@BeforeClass` cannot (Robolectric runs it outside the sandbox classloader). It works, and every Compose test in `src/test` now depends on it. **The gap: `runCatching { runTest {} }` discards whatever it finds.** It cannot distinguish the one known deposit from a genuinely unexpected escaped error, so a future coroutine failure that nobody asserted on becomes **silent** rather than failing a random later test. That trades a loud, misleading symptom for a quiet one. Acceptable as a stopgap because there is exactly one known depositor; not acceptable as the permanent answer, which is why this ticket exists. ### The fix Give the probe hop an **injectable dispatcher**, exactly as `ConversionViewModel`'s constructor already does for `cleanupDispatcher`: ```kotlin class ConversionViewModel @JvmOverloads constructor( app: Application, cleanupDispatcher: CoroutineDispatcher = Dispatchers.IO, // add: probeDispatcher, so a test can put the hop on its own scheduler ) ``` With the hop on a test dispatcher, the error has somewhere to land: it fails the test that caused it, by name, and nothing leaks into the process-wide collector. Then `createDrainedComposeRule()` can lose the drain and become a thin alias — or go away entirely. ### Done means - The dispatcher seam exists and `ConversionViewModelProbeFailureTest` asserts the OOM **at the call** rather than only through the resulting state. - `drainEscapedCoroutineErrors()` is removed, or reduced to something that no longer swallows unknown errors. - **The mutation:** revert the seam, run the full JVM suite twice in a row, and confirm the flake returns — a fix for a heisenbug that cannot be shown to reappear has not been shown to work. - Gate: `./gradlew :app:assembleDebug :app:testDebugUnitTest :app:compileDebugAndroidTestKotlin :app:ktlintCheck :app:detekt :app:lintDebug --continue`
JMR-dev commented 2026-08-24 20:50:50 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Tracked on the board as Backlog, not Ready for Development, and the reason is a sequencing constraint rather than a gap in the evidence:

This is blocked until the #52 chain lands. createDrainedComposeRule() is what every Compose test in src/test currently starts from — #57, and #58, #59, #60, #62, #63 as they arrive. Landing the dispatcher seam first would mean those tests need rewriting onto it mid-flight, and landing it while they are in flight would conflict on ConversionViewModel's constructor.

Order: finish #58–#63, then this, then delete the drain.

The evidence and the done-state are already clear enough for Ready for Development — the flake was observed twice on identical green code, the cause is traced, and the acceptance is written. It is only the ordering that holds it.

Tracked on the board as **Backlog**, not Ready for Development, and the reason is a sequencing constraint rather than a gap in the evidence: **This is blocked until the #52 chain lands.** `createDrainedComposeRule()` is what every Compose test in `src/test` currently starts from — #57, and #58, #59, #60, #62, #63 as they arrive. Landing the dispatcher seam first would mean those tests need rewriting onto it mid-flight, and landing it while they are in flight would conflict on `ConversionViewModel`'s constructor. Order: finish #58–#63, then this, then delete the drain. The evidence and the done-state are already clear enough for Ready for Development — the flake was observed twice on identical green code, the cause is traced, and the acceptance is written. It is only the ordering that holds it.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels bug confirmed sev:medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#66