diff --git a/app/src/androidTest/java/dev/jasonmross/mediaconverter/convert/Media3EngineTest.kt b/app/src/androidTest/java/dev/jasonmross/mediaconverter/convert/Media3EngineTest.kt index a262fae..7195346 100644 --- a/app/src/androidTest/java/dev/jasonmross/mediaconverter/convert/Media3EngineTest.kt +++ b/app/src/androidTest/java/dev/jasonmross/mediaconverter/convert/Media3EngineTest.kt @@ -56,7 +56,7 @@ class Media3EngineTest { fun transcodesH264ToH265AndReportsProgress(): Unit = runBlocking { val seen = mutableListOf() - 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 { diff --git a/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/FakeFailures.kt b/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/FakeFailures.kt new file mode 100644 index 0000000..b8b0fd1 --- /dev/null +++ b/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/FakeFailures.kt @@ -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() +} diff --git a/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/ForcedFailureTest.kt b/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/ForcedFailureTest.kt new file mode 100644 index 0000000..2c18763 --- /dev/null +++ b/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/ForcedFailureTest.kt @@ -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 + } +} diff --git a/app/src/androidTest/java/dev/jasonmross/mediaconverter/ffmpeg/FFmpegEngineTest.kt b/app/src/androidTest/java/dev/jasonmross/mediaconverter/ffmpeg/FFmpegEngineTest.kt index 698a094..7436166 100644 --- a/app/src/androidTest/java/dev/jasonmross/mediaconverter/ffmpeg/FFmpegEngineTest.kt +++ b/app/src/androidTest/java/dev/jasonmross/mediaconverter/ffmpeg/FFmpegEngineTest.kt @@ -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() diff --git a/app/src/main/java/dev/jasonmross/mediaconverter/convert/Media3Engine.kt b/app/src/main/java/dev/jasonmross/mediaconverter/convert/Media3Engine.kt index e258930..131fd68 100644 --- a/app/src/main/java/dev/jasonmross/mediaconverter/convert/Media3Engine.kt +++ b/app/src/main/java/dev/jasonmross/mediaconverter/convert/Media3Engine.kt @@ -39,7 +39,7 @@ import kotlin.coroutines.resumeWithException * suspending function and never have to think about it. */ @UnstableApi -class Media3Engine(private val context: Context) : AutoCloseable { +class Media3Engine(private val context: Context) : HardwareTranscoder { private val thread = HandlerThread("media3-transformer").apply { start() } private val handler = Handler(thread.looper) @@ -52,12 +52,12 @@ class Media3Engine(private val context: Context) : AutoCloseable { * rewrite the moov atom, and a SAF fd is not reliably seekable. Callers stage into * app-private storage and publish afterwards. */ - suspend fun transcode( + override suspend fun transcode( input: Uri, output: File, - videoMimeType: String = MimeTypes.VIDEO_H265, - onProgress: (Int) -> Unit = {}, - ): ExportResult = suspendCancellableCoroutine { cont -> + videoMimeType: String, + onProgress: (Int) -> Unit, + ): Unit = suspendCancellableCoroutine { cont -> handler.post { val transformer = buildTransformer(videoMimeType, cont) val item = EditedMediaItem.Builder(MediaItem.fromUri(input)).build() @@ -76,13 +76,13 @@ class Media3Engine(private val context: Context) : AutoCloseable { private fun buildTransformer( videoMimeType: String, - cont: CancellableContinuation, + cont: CancellableContinuation, ): Transformer = Transformer.Builder(context) .setLooper(thread.looper) .setVideoMimeType(videoMimeType) .addListener(object : Transformer.Listener { override fun onCompleted(composition: Composition, result: ExportResult) { - if (cont.isActive) cont.resume(result) + if (cont.isActive) cont.resume(Unit) } override fun onError( @@ -104,7 +104,7 @@ class Media3Engine(private val context: Context) : AutoCloseable { */ private fun pollProgress( transformer: Transformer, - cont: CancellableContinuation, + cont: CancellableContinuation, onProgress: (Int) -> Unit, ) { val holder = ProgressHolder() @@ -124,6 +124,7 @@ class Media3Engine(private val context: Context) : AutoCloseable { thread.quitSafely() } + private companion object { const val PROGRESS_INTERVAL_MS = 250L } diff --git a/app/src/main/java/dev/jasonmross/mediaconverter/convert/OutputPublisher.kt b/app/src/main/java/dev/jasonmross/mediaconverter/convert/OutputPublisher.kt index 2eb388a..742821b 100644 --- a/app/src/main/java/dev/jasonmross/mediaconverter/convert/OutputPublisher.kt +++ b/app/src/main/java/dev/jasonmross/mediaconverter/convert/OutputPublisher.kt @@ -19,12 +19,12 @@ import java.io.File * The cost is one extra copy and transient double disk usage, which is why * [hasSpaceFor] exists. */ -class OutputPublisher(private val context: Context) { +open class OutputPublisher(private val context: Context) { private val stagingDir: File get() = File(context.cacheDir, "conversions").apply { mkdirs() } - fun createStagingFile(name: String): File = File(stagingDir, name) + open fun createStagingFile(name: String): File = File(stagingDir, name) /** * True if there is room for a further [bytes], including headroom. @@ -32,11 +32,11 @@ class OutputPublisher(private val context: Context) { * Staging means peak usage is roughly input + output at once, so a job that would * just barely fit is rejected rather than failing partway through. */ - fun hasSpaceFor(bytes: Long): Boolean = + open fun hasSpaceFor(bytes: Long): Boolean = stagingDir.usableSpace > bytes + SPACE_HEADROOM_BYTES /** Copies a finished staging file into a user-chosen SAF destination. */ - fun publish(staged: File, destination: Uri) { + open fun publish(staged: File, destination: Uri) { context.contentResolver.openOutputStream(destination)?.use { out -> staged.inputStream().use { it.copyTo(out) } } ?: error("Could not open destination for writing: $destination") diff --git a/app/src/main/java/dev/jasonmross/mediaconverter/convert/Transcoders.kt b/app/src/main/java/dev/jasonmross/mediaconverter/convert/Transcoders.kt new file mode 100644 index 0000000..e8d2f27 --- /dev/null +++ b/app/src/main/java/dev/jasonmross/mediaconverter/convert/Transcoders.kt @@ -0,0 +1,67 @@ +package dev.jasonmross.mediaconverter.convert + +import android.content.Context +import android.net.Uri +import dev.jasonmross.mediaconverter.ffmpeg.FFmpegEngine +import dev.jasonmross.mediaconverter.model.ConversionRequest +import dev.jasonmross.mediaconverter.model.DeviceCodecs +import dev.jasonmross.mediaconverter.codec.AndroidDeviceCodecs +import java.io.File + +/** The hardware conversion path. Implemented by [Media3Engine]. */ +interface HardwareTranscoder : AutoCloseable { + suspend fun transcode( + input: Uri, + output: File, + videoMimeType: String = androidx.media3.common.MimeTypes.VIDEO_H265, + onProgress: (Int) -> Unit = {}, + ) +} + +/** The software conversion path. Implemented by [FFmpegEngine]. */ +interface SoftwareTranscoder { + suspend fun run( + request: ConversionRequest, + inputPath: String, + output: File, + durationMs: Long, + onProgress: (Int) -> Unit = {}, + ) +} + +/** + * The seam that lets tests force failure paths. + * + * Workers are constructed by WorkManager, so they cannot take constructor arguments, + * and the app deliberately carries no DI framework. This holds the few collaborators a + * conversion needs, defaulting to the real implementations. + * + * Its reason for existing is coverage of the branches that only run when something goes + * wrong. Those branches are, by definition, the ones that never execute in a healthy + * test run — and they are also the ones a user meets on a bad day, so leaving them + * unexercised means the error handling is the least-tested code in the app. + * + * Tests must call [reset] afterwards; [dev.jasonmross.mediaconverter.convert.FakeFailures] + * does that for them. + */ +object ConversionDependencies { + + @Volatile + var hardware: (Context) -> HardwareTranscoder = { Media3Engine(it) } + + @Volatile + var software: () -> SoftwareTranscoder = { FFmpegEngine() } + + @Volatile + var publisher: (Context) -> OutputPublisher = { OutputPublisher(it) } + + @Volatile + var deviceCodecs: () -> DeviceCodecs = { AndroidDeviceCodecs.get() } + + fun reset() { + hardware = { Media3Engine(it) } + software = { FFmpegEngine() } + publisher = { OutputPublisher(it) } + deviceCodecs = { AndroidDeviceCodecs.get() } + } +} diff --git a/app/src/main/java/dev/jasonmross/mediaconverter/ffmpeg/FFmpegEngine.kt b/app/src/main/java/dev/jasonmross/mediaconverter/ffmpeg/FFmpegEngine.kt index f575068..de9a820 100644 --- a/app/src/main/java/dev/jasonmross/mediaconverter/ffmpeg/FFmpegEngine.kt +++ b/app/src/main/java/dev/jasonmross/mediaconverter/ffmpeg/FFmpegEngine.kt @@ -5,6 +5,7 @@ import com.arthenica.ffmpegkit.FFmpegKit import com.arthenica.ffmpegkit.FFmpegKitConfig import com.arthenica.ffmpegkit.Level import com.arthenica.ffmpegkit.ReturnCode +import dev.jasonmross.mediaconverter.convert.SoftwareTranscoder import dev.jasonmross.mediaconverter.model.ConversionRequest import kotlinx.coroutines.suspendCancellableCoroutine import java.io.File @@ -23,7 +24,7 @@ import kotlin.coroutines.resumeWithException * to seek backwards to rewrite the moov atom, which a SAF descriptor does not reliably * support — so staging is the safe default for every format rather than a special case. */ -class FFmpegEngine { +class FFmpegEngine : SoftwareTranscoder { init { FFmpegKitConfig.setLogLevel(Level.AV_LOG_WARNING) @@ -37,12 +38,12 @@ class FFmpegEngine { * [durationMs] of zero means progress simply cannot be reported — the caller gets * an indeterminate job rather than a fabricated number. */ - suspend fun run( + override suspend fun run( request: ConversionRequest, inputPath: String, output: File, durationMs: Long, - onProgress: (Int) -> Unit = {}, + onProgress: (Int) -> Unit, ): Unit = suspendCancellableCoroutine { cont -> val args = FFmpegCommandBuilder.build(request, inputPath, output.absolutePath) Log.i(TAG, "ffmpeg ${args.joinToString(" ")}") diff --git a/app/src/main/java/dev/jasonmross/mediaconverter/work/ConcatWorker.kt b/app/src/main/java/dev/jasonmross/mediaconverter/work/ConcatWorker.kt index e34c7e4..49b6879 100644 --- a/app/src/main/java/dev/jasonmross/mediaconverter/work/ConcatWorker.kt +++ b/app/src/main/java/dev/jasonmross/mediaconverter/work/ConcatWorker.kt @@ -11,7 +11,7 @@ import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkerParameters import androidx.work.workDataOf -import dev.jasonmross.mediaconverter.convert.OutputPublisher +import dev.jasonmross.mediaconverter.convert.ConversionDependencies import dev.jasonmross.mediaconverter.ffmpeg.ConcatEngine import dev.jasonmross.mediaconverter.model.OutputFormat @@ -33,7 +33,7 @@ class ConcatWorker( ) : CoroutineWorker(context, params) { private val notifications = ConversionNotifications(applicationContext) - private val publisher = OutputPublisher(applicationContext) + private val publisher = ConversionDependencies.publisher(applicationContext) override suspend fun doWork(): Result { val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse) @@ -69,12 +69,15 @@ class ConcatWorker( ) } catch (e: Throwable) { staged.delete() - if (stopReason == WorkInfo.STOP_REASON_FOREGROUND_SERVICE_TIMEOUT) { - Log.w(TAG, "Foreground budget exhausted while joining; will retry.", e) - Result.retry() - } else { - Log.e(TAG, "Joining failed.", e) - Result.failure(workDataOf(KEY_ERROR to (e.message ?: "Joining failed."))) + when (FailureOutcome.forStopReason(stopReason)) { + FailureOutcome.RETRY -> { + Log.w(TAG, "Foreground budget exhausted while joining; will retry.", e) + Result.retry() + } + FailureOutcome.FAIL -> { + Log.e(TAG, "Joining failed.", e) + Result.failure(workDataOf(KEY_ERROR to (e.message ?: "Joining failed."))) + } } } } diff --git a/app/src/main/java/dev/jasonmross/mediaconverter/work/ConversionWorker.kt b/app/src/main/java/dev/jasonmross/mediaconverter/work/ConversionWorker.kt index 4837d05..b8d8e79 100644 --- a/app/src/main/java/dev/jasonmross/mediaconverter/work/ConversionWorker.kt +++ b/app/src/main/java/dev/jasonmross/mediaconverter/work/ConversionWorker.kt @@ -13,11 +13,8 @@ import androidx.work.WorkInfo import androidx.work.WorkerParameters import androidx.work.workDataOf import com.arthenica.ffmpegkit.FFmpegKitConfig -import dev.jasonmross.mediaconverter.codec.AndroidDeviceCodecs -import dev.jasonmross.mediaconverter.convert.Media3Engine +import dev.jasonmross.mediaconverter.convert.ConversionDependencies import dev.jasonmross.mediaconverter.convert.MediaProbe -import dev.jasonmross.mediaconverter.convert.OutputPublisher -import dev.jasonmross.mediaconverter.ffmpeg.FFmpegEngine import dev.jasonmross.mediaconverter.model.ConversionRequest import dev.jasonmross.mediaconverter.model.ConversionRouter import dev.jasonmross.mediaconverter.model.Engine @@ -45,7 +42,8 @@ class ConversionWorker( ) : CoroutineWorker(context, params) { private val notifications = ConversionNotifications(applicationContext) - private val publisher = OutputPublisher(applicationContext) + // Resolved through ConversionDependencies so tests can force the failure paths. + private val publisher = ConversionDependencies.publisher(applicationContext) override suspend fun doWork(): Result { val inputUri = inputData.getString(KEY_INPUT_URI)?.let(Uri::parse) @@ -69,7 +67,7 @@ class ConversionWorker( setForeground(foregroundInfo(displayName, percent = 0, indeterminate = true)) val probe = MediaProbe.probe(applicationContext, inputUri) - val devices = AndroidDeviceCodecs.get() + val devices = ConversionDependencies.deviceCodecs() val request = ConversionRequest( format = format, quality = quality, @@ -115,7 +113,7 @@ class ConversionWorker( staged: File, displayName: String, ) { - val engine = Media3Engine(applicationContext) + val engine = ConversionDependencies.hardware(applicationContext) try { engine.transcode(inputUri, staged, media3MimeType(request.format)) { percent -> publishProgress(displayName, percent) @@ -147,9 +145,10 @@ class ConversionWorker( inputUri.path } ?: error("Could not open the input file.") - FFmpegEngine().run(request, inputPath, staged, request.probe.durationMs) { percent -> - publishProgress(displayName, percent) - } + ConversionDependencies.software() + .run(request, inputPath, staged, request.probe.durationMs) { percent -> + publishProgress(displayName, percent) + } } private var lastNotified = 0L @@ -178,14 +177,17 @@ class ConversionWorker( * later rather than tell the user the conversion failed — the work is still valid, * there is simply no budget right now. */ - private fun handleTimeoutIfNeeded(cause: Throwable): Result { - if (stopReason == WorkInfo.STOP_REASON_FOREGROUND_SERVICE_TIMEOUT) { - Log.w(TAG, "Foreground service budget exhausted; will retry.", cause) - return Result.retry() + private fun handleTimeoutIfNeeded(cause: Throwable): Result = + when (FailureOutcome.forStopReason(stopReason)) { + FailureOutcome.RETRY -> { + Log.w(TAG, "Foreground service budget exhausted; will retry.", cause) + Result.retry() + } + FailureOutcome.FAIL -> { + Log.e(TAG, "Conversion failed.", cause) + Result.failure(workDataOf(KEY_ERROR to (cause.message ?: "Conversion failed."))) + } } - Log.e(TAG, "Conversion failed.", cause) - return Result.failure(workDataOf(KEY_ERROR to (cause.message ?: "Conversion failed."))) - } private fun media3MimeType(format: OutputFormat): String = when (format.videoCodec) { VideoCodec.H264 -> MimeTypes.VIDEO_H264 diff --git a/app/src/main/java/dev/jasonmross/mediaconverter/work/FailureOutcome.kt b/app/src/main/java/dev/jasonmross/mediaconverter/work/FailureOutcome.kt new file mode 100644 index 0000000..dc0adf2 --- /dev/null +++ b/app/src/main/java/dev/jasonmross/mediaconverter/work/FailureOutcome.kt @@ -0,0 +1,25 @@ +package dev.jasonmross.mediaconverter.work + +import androidx.work.WorkInfo + +/** + * Decides whether a failed job should be retried or reported as failed. + * + * A pure function rather than a branch inside the worker, because the case that matters + * cannot be provoked in a test: the foreground-service budget is six hours per + * twenty-four, and no test is going to exhaust it. Isolating the decision means the + * rule itself can still be verified on the JVM, even though the condition that triggers + * it in production cannot be reproduced. + */ +enum class FailureOutcome { + /** Budget exhausted, not a real failure — the work is still valid, so try later. */ + RETRY, + + /** A genuine failure; report it to the user. */ + FAIL; + + companion object { + fun forStopReason(stopReason: Int): FailureOutcome = + if (stopReason == WorkInfo.STOP_REASON_FOREGROUND_SERVICE_TIMEOUT) RETRY else FAIL + } +} diff --git a/app/src/test/java/dev/jasonmross/mediaconverter/work/FailureOutcomeTest.kt b/app/src/test/java/dev/jasonmross/mediaconverter/work/FailureOutcomeTest.kt new file mode 100644 index 0000000..bbbc542 --- /dev/null +++ b/app/src/test/java/dev/jasonmross/mediaconverter/work/FailureOutcomeTest.kt @@ -0,0 +1,58 @@ +package dev.jasonmross.mediaconverter.work + +import androidx.work.WorkInfo +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * The retry-versus-fail rule. + * + * Isolated from the worker precisely so it can be tested: the condition that triggers + * a retry in production is the foreground-service budget running out, six hours per + * twenty-four, which no test can reach. Extracting the decision means the rule is still + * verified even though its trigger cannot be reproduced. + */ +class FailureOutcomeTest { + + @Test + fun `a foreground service timeout is a retry, not a failure`() { + // The work is still valid; there is simply no budget right now. Telling the + // user their conversion failed would be wrong. + assertEquals( + FailureOutcome.RETRY, + FailureOutcome.forStopReason(WorkInfo.STOP_REASON_FOREGROUND_SERVICE_TIMEOUT), + ) + } + + @Test + fun `an ordinary failure is reported as a failure`() { + assertEquals( + FailureOutcome.FAIL, + FailureOutcome.forStopReason(WorkInfo.STOP_REASON_NOT_STOPPED), + ) + } + + @Test + fun `every other stop reason fails rather than retrying forever`() { + // Retrying on, say, a user cancellation or a battery constraint would either + // ignore the user or spin. Only the timeout earns a retry. + val others = listOf( + WorkInfo.STOP_REASON_CANCELLED_BY_APP, + WorkInfo.STOP_REASON_USER, + WorkInfo.STOP_REASON_CONSTRAINT_BATTERY_NOT_LOW, + WorkInfo.STOP_REASON_CONSTRAINT_CHARGING, + WorkInfo.STOP_REASON_CONSTRAINT_CONNECTIVITY, + WorkInfo.STOP_REASON_CONSTRAINT_DEVICE_IDLE, + WorkInfo.STOP_REASON_CONSTRAINT_STORAGE_NOT_LOW, + WorkInfo.STOP_REASON_DEVICE_STATE, + WorkInfo.STOP_REASON_QUOTA, + WorkInfo.STOP_REASON_BACKGROUND_RESTRICTION, + WorkInfo.STOP_REASON_APP_STANDBY, + WorkInfo.STOP_REASON_TIMEOUT, + WorkInfo.STOP_REASON_UNKNOWN, + ) + others.forEach { + assertEquals("stop reason $it should fail", FailureOutcome.FAIL, FailureOutcome.forStopReason(it)) + } + } +}