Add a seam for forcing failure paths, and force thirteen of them

Error handling was the least-tested code in the app. It only runs when something goes wrong, which is exactly what a healthy test run avoids, so the branches a user meets on a bad day were the ones that had never executed. An audit put it at 4 of 18 fallback branches actually forced by a test. There was also no mechanism to do better: ConversionWorker constructed Media3Engine and FFmpegEngine directly, so no test could make either of them fail. Media3Engine and FFmpegEngine now implement HardwareTranscoder and SoftwareTranscoder, and ConversionDependencies holds the factories. Workers are built by WorkManager and the app deliberately carries no DI framework, so a small settable holder is the least machinery that does the job. The foreground-service timeout needed different treatment. Its trigger is the six-hour-per-day budget expiring, which no test can reach, so the decision moved out of the worker into FailureOutcome. The rule is now verified on the JVM across every WorkInfo stop reason, including that only the timeout earns a retry -- retrying a user cancellation would ignore the user, and retrying a constraint failure would spin. Thirteen branches are now forced, including both free-space prechecks, both engines failing, a missing input, and the router bypassing hardware entirely for MP3. The dynamic fallback is covered twice over: once by injection, and once by HardwareFallbackTest driving it with genuinely undecodable 4:4:4 footage. Injection alone would prove the plumbing without proving the condition ever arises in reality. Five remain unforced and are listed in ConversionDependencies' documentation. They need a SAF grant to be revoked mid-job, which is not something a test can arrange. Media3EngineTest now asserts against the muxed file rather than the engine's own ExportResult, which is a stronger check anyway: a result object can report success for a file that will not play. 35 instrumented tests on a Pixel 10 Pro XL and 66 unit tests, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-20 09:54:06 -05:00
parent 356c04d137
commit 83fb55515b
12 changed files with 474 additions and 44 deletions
@@ -56,7 +56,7 @@ class Media3EngineTest {
    fun transcodesH264ToH265AndReportsProgress(): Unit = runBlocking {
        val seen = mutableListOf<Int>()

-        val result = engine.transcode(
+        engine.transcode(
            input = Uri.fromFile(input),
            output = output,
            videoMimeType = MimeTypes.VIDEO_H265,
@@ -67,8 +67,9 @@ class Media3EngineTest {
        // Assert against the muxed file, not just the reported result: this is what
        // actually proves the output is HEVC rather than a silent fallback to H.264.
        assertEquals(MimeTypes.VIDEO_H265, videoMimeTypeOf(output))
-        assertTrue("no duration reported", result.approximateDurationMs > 0)
-        assertTrue("no frames encoded", result.videoFrameCount > 0)
+        // Assert against the muxed file rather than the engine's own report: a result
+        // object can claim success for a file that will not play.
+        assertTrue("output has no duration", durationMsOf(output) > 0)
        // Deliberately NOT asserting that progress fired. Polling is on a 250 ms tick,
        // and a 3 s 320x240 clip can finish inside one tick on fast hardware, which
        // would make the assertion fail intermittently for no real defect.
@@ -107,6 +108,19 @@ class Media3EngineTest {
        }
    }

+    private fun durationMsOf(file: File): Long {
+        val extractor = MediaExtractor()
+        return try {
+            extractor.setDataSource(file.absolutePath)
+            (0 until extractor.trackCount)
+                .map { extractor.getTrackFormat(it) }
+                .filter { it.containsKey(MediaFormat.KEY_DURATION) }
+                .maxOfOrNull { it.getLong(MediaFormat.KEY_DURATION) / 1000 } ?: 0L
+        } finally {
+            extractor.release()
+        }
+    }
+
    private fun videoMimeTypeOf(file: File): String? {
        val extractor = MediaExtractor()
        try {
@@ -0,0 +1,74 @@
+package dev.jasonmross.mediaconverter.fallback
+
+import android.content.Context
+import android.net.Uri
+import dev.jasonmross.mediaconverter.convert.ConversionDependencies
+import dev.jasonmross.mediaconverter.convert.HardwareTranscoder
+import dev.jasonmross.mediaconverter.convert.OutputPublisher
+import dev.jasonmross.mediaconverter.convert.SoftwareTranscoder
+import dev.jasonmross.mediaconverter.model.ConversionRequest
+import java.io.File
+
+/**
+ * Test doubles that force the failure paths.
+ *
+ * These exist because error handling is otherwise the least-tested code in the app: by
+ * definition it only runs when something goes wrong, which is exactly what a healthy
+ * test run avoids. Without a way to inject failure, the branches a user meets on a bad
+ * day are the ones that were never executed.
+ */
+object FakeFailures {
+
+    class ExplodingHardware(private val message: String = "hardware exploded") : HardwareTranscoder {
+        var called = false
+        override suspend fun transcode(
+            input: Uri,
+            output: File,
+            videoMimeType: String,
+            onProgress: (Int) -> Unit,
+        ) {
+            called = true
+            throw IllegalStateException(message)
+        }
+
+        override fun close() = Unit
+    }
+
+    class ExplodingSoftware(private val message: String = "software exploded") : SoftwareTranscoder {
+        var called = false
+        override suspend fun run(
+            request: ConversionRequest,
+            inputPath: String,
+            output: File,
+            durationMs: Long,
+            onProgress: (Int) -> Unit,
+        ) {
+            called = true
+            throw IllegalStateException(message)
+        }
+    }
+
+    /** Records that it ran and writes a plausible output, without doing real work. */
+    class RecordingSoftware : SoftwareTranscoder {
+        var called = false
+        override suspend fun run(
+            request: ConversionRequest,
+            inputPath: String,
+            output: File,
+            durationMs: Long,
+            onProgress: (Int) -> Unit,
+        ) {
+            called = true
+            output.parentFile?.mkdirs()
+            output.writeBytes(ByteArray(1024))
+            onProgress(100)
+        }
+    }
+
+    class FullDisk(context: Context) : OutputPublisher(context) {
+        override fun hasSpaceFor(bytes: Long): Boolean = false
+    }
+
+    /** Restores the real implementations. Always call this from @After. */
+    fun reset() = ConversionDependencies.reset()
+}
@@ -0,0 +1,185 @@
+package dev.jasonmross.mediaconverter.fallback
+
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import androidx.work.WorkInfo
+import androidx.work.WorkManager
+import dev.jasonmross.mediaconverter.convert.ConversionDependencies
+import dev.jasonmross.mediaconverter.model.Engine
+import dev.jasonmross.mediaconverter.model.OutputFormat
+import dev.jasonmross.mediaconverter.model.QualityTier
+import dev.jasonmross.mediaconverter.work.ConcatWorker
+import dev.jasonmross.mediaconverter.work.ConversionWorker
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import java.io.File
+
+/**
+ * Forces each failure path that real media cannot reliably trigger.
+ *
+ * Paired with [HardwareFallbackTest], which drives the same fallback with a genuinely
+ * undecodable file. This one covers the branches that would otherwise need a full disk,
+ * a broken codec, or a six-hour foreground-service budget to reach.
+ */
+@UnstableApi
+@RunWith(AndroidJUnit4::class)
+class ForcedFailureTest {
+
+    private val context = InstrumentationRegistry.getInstrumentation().targetContext
+    private val workManager = WorkManager.getInstance(context)
+    private lateinit var input: File
+
+    @Before
+    fun setUp() {
+        input = File(context.cacheDir, SAMPLE)
+        InstrumentationRegistry.getInstrumentation().context.assets
+            .open(SAMPLE)
+            .use { asset -> input.outputStream().use { asset.copyTo(it) } }
+    }
+
+    @After
+    fun tearDown() {
+        FakeFailures.reset()
+        input.delete()
+        File(context.cacheDir, "conversions").listFiles()?.forEach { it.delete() }
+    }
+
+    private suspend fun runToCompletion(request: androidx.work.OneTimeWorkRequest): WorkInfo? {
+        workManager.enqueue(request).result.get()
+        return withTimeout(TIMEOUT_MS) {
+            workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
+        }
+    }
+
+    private fun convertRequest(format: OutputFormat = OutputFormat.MP4_H265) =
+        ConversionWorker.request(
+            inputUri = Uri.fromFile(input),
+            displayName = SAMPLE,
+            sizeBytes = input.length(),
+            format = format,
+            quality = QualityTier.FAST,
+        )
+
+    // --- the dynamic fallback, forced rather than provoked -------------------
+
+    @Test
+    fun hardwareFailureFallsBackToSoftware(): Unit = runBlocking {
+        val hardware = FakeFailures.ExplodingHardware()
+        val software = FakeFailures.RecordingSoftware()
+        ConversionDependencies.hardware = { hardware }
+        ConversionDependencies.software = { software }
+
+        val terminal = runToCompletion(convertRequest(OutputFormat.MP4_H264))
+
+        assertEquals(WorkInfo.State.SUCCEEDED, terminal?.state)
+        assertTrue("the hardware path should have been attempted", hardware.called)
+        assertTrue("the software path should have rescued it", software.called)
+    }
+
+    @Test
+    fun whenBothEnginesFailTheJobFailsWithTheReason(): Unit = runBlocking {
+        ConversionDependencies.hardware = { FakeFailures.ExplodingHardware() }
+        ConversionDependencies.software = { FakeFailures.ExplodingSoftware("no codec available") }
+
+        val terminal = runToCompletion(convertRequest(OutputFormat.MP4_H264))
+
+        assertEquals(WorkInfo.State.FAILED, terminal?.state)
+        assertEquals(
+            "the user should see why it failed",
+            "no codec available",
+            terminal?.outputData?.getString(ConversionWorker.KEY_ERROR),
+        )
+    }
+
+    @Test
+    fun aJobRoutedStraightToFfmpegDoesNotTouchTheHardwarePath(): Unit = runBlocking {
+        val hardware = FakeFailures.ExplodingHardware()
+        val software = FakeFailures.RecordingSoftware()
+        ConversionDependencies.hardware = { hardware }
+        ConversionDependencies.software = { software }
+
+        // MP3 has no Android encoder at all, so the router must bypass Media3 entirely.
+        val terminal = runToCompletion(convertRequest(OutputFormat.MP3))
+
+        assertEquals(WorkInfo.State.SUCCEEDED, terminal?.state)
+        assertEquals(
+            Engine.FFMPEG.name,
+            terminal?.outputData?.getString(ConversionWorker.KEY_ENGINE_USED),
+        )
+        assertTrue(!hardware.called, "the hardware path must not be attempted for MP3")
+        assertTrue("software should have run", software.called)
+    }
+
+    // --- the free-space precheck -------------------------------------------
+
+    @Test
+    fun aFullDiskFailsBeforeAnyConversionStarts(): Unit = runBlocking {
+        val hardware = FakeFailures.ExplodingHardware()
+        ConversionDependencies.publisher = { FakeFailures.FullDisk(it) }
+        ConversionDependencies.hardware = { hardware }
+
+        val terminal = runToCompletion(convertRequest())
+
+        assertEquals(WorkInfo.State.FAILED, terminal?.state)
+        assertTrue(
+            "the message should mention space, was: " +
+                terminal?.outputData?.getString(ConversionWorker.KEY_ERROR),
+            terminal?.outputData?.getString(ConversionWorker.KEY_ERROR)
+                .orEmpty().contains("space", ignoreCase = true),
+        )
+        assertTrue(
+            "no conversion should be attempted when the disk is full",
+            !hardware.called,
+        )
+    }
+
+    @Test
+    fun aFullDiskFailsAJoinBeforeItStarts(): Unit = runBlocking {
+        ConversionDependencies.publisher = { FakeFailures.FullDisk(it) }
+
+        val request = ConcatWorker.request(
+            inputs = listOf(Uri.fromFile(input), Uri.fromFile(input)),
+            totalBytes = input.length() * 2,
+        )
+        val terminal = runToCompletion(request)
+
+        assertEquals(WorkInfo.State.FAILED, terminal?.state)
+        assertTrue(
+            terminal?.outputData?.getString(ConcatWorker.KEY_ERROR)
+                .orEmpty().contains("space", ignoreCase = true),
+        )
+    }
+
+    // --- malformed input ----------------------------------------------------
+
+    @Test
+    fun aMissingInputFailsRatherThanCrashing(): Unit = runBlocking {
+        val request = ConversionWorker.request(
+            inputUri = Uri.fromFile(File(context.cacheDir, "does_not_exist.mp4")),
+            displayName = "does_not_exist.mp4",
+            sizeBytes = 1,
+        )
+        val terminal = runToCompletion(request)
+        assertEquals(WorkInfo.State.FAILED, terminal?.state)
+    }
+
+    private fun assertTrue(message: String, condition: Boolean) =
+        org.junit.Assert.assertTrue(message, condition)
+
+    private fun assertTrue(condition: Boolean, message: String) =
+        org.junit.Assert.assertTrue(message, condition)
+
+    private companion object {
+        const val SAMPLE = "sample_h264.mp4"
+        const val TIMEOUT_MS = 300_000L
+    }
+}
@@ -28,7 +28,7 @@ import java.io.File
 class FFmpegEngineTest {

    private val context = InstrumentationRegistry.getInstrumentation().targetContext
-    private val engine = FFmpegEngine()
+    private val engine: dev.jasonmross.mediaconverter.convert.SoftwareTranscoder = FFmpegEngine()
    private lateinit var input: File
    private val outputs = mutableListOf<File>()