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 7195346..6bffa01 100644 --- a/app/src/androidTest/java/dev/jasonmross/mediaconverter/convert/Media3EngineTest.kt +++ b/app/src/androidTest/java/dev/jasonmross/mediaconverter/convert/Media3EngineTest.kt @@ -8,6 +8,7 @@ import androidx.media3.common.util.UnstableApi import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue @@ -108,6 +109,32 @@ class Media3EngineTest { } } + /** + * Transformer.start() failing synchronously. + * + * Previously written off as Media3-internal and unreachable. It is not: an output + * path whose parent directory does not exist makes start() fail, and the engine has + * to surface that as a rejected suspension rather than hanging forever waiting for + * a listener callback that will never come. A hang here would be far worse than an + * exception, because the worker would sit holding a foreground service. + */ + @Test + fun anUnwritableOutputPathFailsInsteadOfHanging() { + val impossible = File("/does/not/exist/nested/out.mp4") + val failure = runCatching { + runBlocking { + withTimeout(30_000) { + engine.transcode(Uri.fromFile(input), impossible, MimeTypes.VIDEO_H265) {} + } + } + }.exceptionOrNull() + + assertTrue( + "an unwritable output must raise, not hang or silently pass; got $failure", + failure != null && failure !is kotlinx.coroutines.TimeoutCancellationException, + ) + } + private fun durationMsOf(file: File): Long { val extractor = MediaExtractor() return try { diff --git a/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/UnopenableUriTest.kt b/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/UnopenableUriTest.kt new file mode 100644 index 0000000..d5afd72 --- /dev/null +++ b/app/src/androidTest/java/dev/jasonmross/mediaconverter/fallback/UnopenableUriTest.kt @@ -0,0 +1,135 @@ +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.Data +import androidx.work.OneTimeWorkRequestBuilder +import androidx.work.WorkInfo +import androidx.work.WorkManager +import dev.jasonmross.mediaconverter.convert.OutputPublisher +import dev.jasonmross.mediaconverter.ffmpeg.ConcatEngine +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 + +/** + * The branches that handle a URI the app cannot open. + * + * These were previously written off as needing a SAF grant to be revoked mid-job. That + * was wrong: a URI that was *never* valid reaches exactly the same null branch as one + * whose permission was withdrawn, and pointing at a provider that does not exist is + * trivial to arrange. + * + * The assertions deliberately check the *outcome* rather than a specific exception + * type. Whether the resolver returns null or throws is an implementation detail of the + * provider; what matters is that the job fails cleanly and tells the user something, + * instead of crashing the worker. + */ +@UnstableApi +@RunWith(AndroidJUnit4::class) +class UnopenableUriTest { + + private val context = InstrumentationRegistry.getInstrumentation().targetContext + private val workManager = WorkManager.getInstance(context) + private lateinit var staged: File + + /** A syntactically valid content URI whose authority does not exist. */ + private val bogus: Uri = Uri.parse("content://dev.jasonmross.nonexistent.provider/media/1") + + @Before + fun setUp() { + staged = File(context.cacheDir, "unopenable_src.bin").apply { writeBytes(ByteArray(2048)) } + } + + @After + fun tearDown() { + FakeFailures.reset() + staged.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 } + } + } + + // --- 1. conversion input that cannot be opened --------------------------- + + @Test + fun aConversionInputThatCannotBeOpenedFailsCleanly(): Unit = runBlocking { + val request = ConversionWorker.request( + inputUri = bogus, + displayName = "gone.mp4", + sizeBytes = 1024, + ) + val terminal = runToCompletion(request) + + assertEquals(WorkInfo.State.FAILED, terminal?.state) + assertTrue( + "the failure should carry a message for the user", + !terminal?.outputData?.getString(ConversionWorker.KEY_ERROR).isNullOrBlank(), + ) + } + + // --- 2. destination that cannot be written ------------------------------- + + @Test + fun publishingToAnUnwritableDestinationThrowsRatherThanSilentlySucceeding() { + val publisher = OutputPublisher(context) + val failure = runCatching { publisher.publish(staged, bogus) }.exceptionOrNull() + assertTrue( + "publishing to a dead provider must not appear to succeed, got $failure", + failure != null, + ) + } + + // --- 3. concat input that cannot be opened ------------------------------- + + @Test + fun aJoinInputThatCannotBeOpenedFailsCleanly(): Unit = runBlocking { + val out = File(context.cacheDir, "joined_unopenable.mp4").also { it.delete() } + val failure = runCatching { + ConcatEngine(context).join(listOf(bogus, bogus), out) + }.exceptionOrNull() + assertTrue("joining unopenable inputs must fail, got $failure", failure != null) + out.delete() + } + + // --- 4. the concat worker's missing-input-array branch ------------------- + + @Test + fun aJoinWithNoInputArrayFailsWithAMessage(): Unit = runBlocking { + // ConcatWorker.request() always sets the array, so this branch is unreachable + // through it -- but a worker is just input Data, and WorkManager will happily + // run one built without it. That is also what a corrupted or version-skewed + // queue entry would look like after an app update. + val request = OneTimeWorkRequestBuilder() + .setInputData(Data.Builder().putLong(ConcatWorker.KEY_TOTAL_BYTES, 1).build()) + .build() + + val terminal = runToCompletion(request) + + assertEquals(WorkInfo.State.FAILED, terminal?.state) + assertEquals( + "No input files.", + terminal?.outputData?.getString(ConcatWorker.KEY_ERROR), + ) + } + + private companion object { + const val TIMEOUT_MS = 300_000L + } +} diff --git a/app/src/main/java/dev/jasonmross/mediaconverter/convert/Transcoders.kt b/app/src/main/java/dev/jasonmross/mediaconverter/convert/Transcoders.kt index e8d2f27..3e7cfcf 100644 --- a/app/src/main/java/dev/jasonmross/mediaconverter/convert/Transcoders.kt +++ b/app/src/main/java/dev/jasonmross/mediaconverter/convert/Transcoders.kt @@ -41,8 +41,13 @@ interface SoftwareTranscoder { * 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] + * Tests must call [reset] afterwards; `FakeFailures` in the androidTest source set * does that for them. + * + * Every failure branch in the conversion and join paths is now forced by a test. The + * three that needed a seam are covered here; the rest are reachable directly, either + * with a content URI pointing at a provider that does not exist, or by constructing a + * worker's input Data by hand rather than through its request() helper. */ object ConversionDependencies {