From bc8e67888eea2c2445180ed1570ed5a5bae55a4d Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Sun, 6 Sep 2026 01:34:17 -0500 Subject: [PATCH] Wait for the pick the launcher test is about (#220) onInputPicked does not reach Ready on the calling thread. It hops twice -- withContext(pickDispatcher) { InputQuery.describe(...) } and then the probe -- and pickDispatcher defaults to Dispatchers.IO, a real background thread Compose's idling knows nothing about. So deliver() returned with the state still Idle, and asserting immediately was a race the test usually won. It lost five times on CI in one day, on PRs whose diffs were instrumented tests and documentation and could not reach it. Two of those failures came alongside #125's deadlock and could be argued as fallout; three did not. waitUntil polls through waitForIdle, draining the main looper each time, so it sees the recomposition the IO hop eventually posts back. Injecting the dispatcher would be better and is not available here. pickDispatcher is a constructor parameter precisely so a test can pin it, but this test composes the real ConverterScreen, which resolves its own ViewModel through viewModel() -- the seam is one layer below the launcher edge this class exists to cover, and reaching for it would mean not testing that edge. The evidence is the mutation rather than the repetition count, per the note #218 left: transposing the two launcher callbacks at ConverterScreen.kt:70 and :83 -- the exact defect this test guards -- still fails it, so the wait did not make it vacuous. A transposed callback leaves the screen in Idle forever and it fails on the timeout with the meaning it had before. Supporting evidence, eight consecutive green runs of the class. Co-Authored-By: Claude Opus 5 (1M context) --- .../convert/LauncherWiringTest.kt | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/app/src/test/java/org/libremediaconverter/convert/LauncherWiringTest.kt b/app/src/test/java/org/libremediaconverter/convert/LauncherWiringTest.kt index 0e38b1e..b939fc3 100644 --- a/app/src/test/java/org/libremediaconverter/convert/LauncherWiringTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/LauncherWiringTest.kt @@ -6,6 +6,7 @@ import android.net.Uri import androidx.activity.ComponentActivity import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithTag import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick import androidx.media3.common.util.UnstableApi @@ -77,6 +78,28 @@ class LauncherWiringTest { * The transposition guard. A picked file has to reach `onInputPicked`, which is observable as * the screen arriving at `Ready` with the file card showing — `save()` from `Idle` returns at * its own guard and leaves nothing behind. + * + * ## Why this waits rather than asserting straight away (#220) + * + * `onInputPicked` does not reach `Ready` on the calling thread. It hops twice — + * `withContext(pickDispatcher) { InputQuery.describe(...) }` and then the probe — and + * `pickDispatcher` defaults to `Dispatchers.IO`, a real background thread that Compose's + * idling does not know about. `deliver` therefore returns with the state still `Idle` more + * often than not, and asserting immediately was a race the test usually won. + * + * It lost five times on CI in one day, on PRs whose diffs were instrumented tests and + * documentation, which is what #220 was filed for. `waitUntil` polls through + * `waitForIdle`, so it drains the main looper each time round and sees the recomposition that + * the IO hop eventually posts back. + * + * **Injecting the dispatcher would be better and is not available here.** `pickDispatcher` is + * a constructor parameter precisely so a test can pin it, but this test composes the real + * `ConverterScreen`, which resolves its own ViewModel through `viewModel()` — the seam exists + * one layer below the thing under test. Pinning it would mean not testing the launcher edge, + * which is the whole point of this class. + * + * The wait does not weaken the assertion: transposing the two callbacks leaves the screen in + * `Idle` forever, so it fails on the timeout with the same meaning it failed with before. */ @Test fun `a picked document is loaded as input rather than saved to`() { @@ -85,6 +108,11 @@ class LauncherWiringTest { composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).performClick() deliver(Uri.parse("content://test/holiday.mkv")) + composeRule.waitUntil(PICK_TIMEOUT_MS) { + composeRule.onAllNodesWithTag(TestTags.Converter.FILE_CARD_NAME) + .fetchSemanticsNodes() + .isNotEmpty() + } composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertIsDisplayed() } @@ -138,4 +166,13 @@ class LauncherWiringTest { ) composeRule.waitForIdle() } + + private companion object { + /** + * Long enough that a slow CI runner is not the reason this fails, short enough that a + * genuinely transposed callback does not stall the suite. The pick normally lands in + * single-digit milliseconds. + */ + const val PICK_TIMEOUT_MS = 10_000L + } }