From 4ff44be1d7942b0cc9b01ec5941ce03fa5f6d1d7 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Mon, 24 Aug 2026 22:45:02 -0500 Subject: [PATCH] Give the Robolectric choice a reason that is still true Two test classes justified using Robolectric by asserting that the alternative does not exist: AppRootRestorationTest "The instrumented tests cannot run on the development host at all (see CLAUDE.md)" OutputPublisherStagingTest "The instrumented suite cannot run on the development host, so this is the only place [it] can be caught" Both were true when written and stopped being true on 2026-08-22, when the segfault was traced to SwiftShader's Reactor JIT against SELinux execheap rather than to the machine. tools/local-emulator/run-e2e.sh has run API 33-36 here since. The first one cites CLAUDE.md as its authority, and PR #73 corrected CLAUDE.md to say the opposite. So it was no longer merely stale: a reader who followed the reference found the contradiction, with the citation making the wrong half look verified. That is the worst version of this -- R14, R15, R20 and R25 were all the same defect, and this is the fifth. The choice itself was never wrong, which is why the fix is not to move these tests. Both belong on the JVM, and the honest reason is cost rather than impossibility: neither needs anything a device supplies, and both run inside the same ./gradlew invocation as every other unit test instead of booting an emulator. That argument survives the correction; the premise did not. The old line also has a second failure mode worth naming. "Nobody can execute this" invites a reader to skip the local run and let CI decide, which is the opposite of what the definition-of-done in #51 asks for. Verified: the string appears nowhere in app/src now, and testDebugUnitTest, ktlintCheck and detekt are green. Closes #46. --- .../org/libremediaconverter/AppRootRestorationTest.kt | 9 ++++++--- .../convert/OutputPublisherStagingTest.kt | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt b/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt index 07dd1f0..5c125b7 100644 --- a/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt +++ b/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt @@ -33,9 +33,12 @@ import org.robolectric.RobolectricTestRunner * representation survives a `Bundle` round trip. A JVM round-trip test on the * saver covers the representation. * - * Robolectric rather than the instrumented suite, deliberately. The instrumented tests - * cannot run on the development host at all (see CLAUDE.md), and a red test nobody can - * execute is not a loop anyone can work in. + * Robolectric rather than the instrumented suite, deliberately -- but not because the + * instrumented suite is unavailable. It runs on this host for API 33-36 + * (`tools/local-emulator/run-e2e.sh`), and CI runs 33-37. The reason is cost: this test + * needs a composition and a saved-state round trip, nothing a device supplies, and it runs + * in the same `./gradlew` invocation as every other JVM test instead of booting an + * emulator. A loop measured in seconds is a loop people stay inside. */ @UnstableApi @RunWith(RobolectricTestRunner::class) diff --git a/app/src/test/java/org/libremediaconverter/convert/OutputPublisherStagingTest.kt b/app/src/test/java/org/libremediaconverter/convert/OutputPublisherStagingTest.kt index 4419919..f778f5a 100644 --- a/app/src/test/java/org/libremediaconverter/convert/OutputPublisherStagingTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/OutputPublisherStagingTest.kt @@ -18,8 +18,10 @@ import java.util.UUID * the actual filesystem — the same calls `reset()` makes, without needing a ViewModel (both * of those construct a `WorkManager`, which is not initialised on the JVM classpath). * - * The instrumented suite cannot run on the development host, so this is the only place the - * "Start over leaks a full-size copy" defect can be caught before CI. + * The instrumented suite could also catch the "Start over leaks a full-size copy" defect -- + * it runs on this host for API 33-36 (`tools/local-emulator/run-e2e.sh`) and on CI for + * 33-37. Here rather than there because a real `cacheDir` is all the defect needs, and + * finding it costs an emulator boot there and a few seconds here. */ @RunWith(RobolectricTestRunner::class) class OutputPublisherStagingTest {