diff --git a/app/src/test/java/org/libremediaconverter/convert/Media3MuxerGuardTest.kt b/app/src/test/java/org/libremediaconverter/convert/Media3MuxerGuardTest.kt new file mode 100644 index 0000000..8207986 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/Media3MuxerGuardTest.kt @@ -0,0 +1,99 @@ +package org.libremediaconverter.convert + +import android.net.Uri +import androidx.media3.common.util.UnstableApi +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.model.AudioCodec +import org.libremediaconverter.model.AudioPlan +import org.libremediaconverter.model.Container +import org.libremediaconverter.model.ConversionRequest +import org.libremediaconverter.model.CopyPlanner +import org.libremediaconverter.model.InputProbe +import org.libremediaconverter.model.OutputSpec +import org.libremediaconverter.model.VideoCodec +import org.libremediaconverter.model.VideoPlan +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import java.io.File +import java.util.concurrent.CancellationException + +/** + * A job that reached Media3 with a container Media3 cannot mux. + * + * [Media3Muxers]' own KDoc names the defect this guards: *"the router claimed five containers while + * the engine silently wrote MP4 for all of them."* `factoryFor` answers null for fourteen of the + * app's containers, and `buildTransformer` turns that null into a failed job rather than letting + * `Transformer` fall back to its default muxer. + * + * The guard had never fired. `Media3Engine$buildTransformer$3` -- the `requireNotNull` message + * lambda -- was four lines and four branches at 0%, which is to say the entire repair for a defect + * the codebase went to the trouble of writing down was untested. Weakening it would restore that + * bug silently, because the wrong output is a *playable file with the wrong container*, not a crash. + * + * Same harness and same two disciplines as [Media3EngineEmptyCompositionTest]: assert the plan + * really is the one the test needs before driving the engine, and rule out + * `CancellationException` so an unresumed continuation cannot read as a pass. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class Media3MuxerGuardTest { + + @Test + fun `a container Media3 cannot mux fails the job rather than silently writing MP4`() { + val context = RuntimeEnvironment.getApplication() + val engine = Media3Engine(context) + val request = ConversionRequest( + spec = OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.OPUS), + probe = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MP4), + ) + + // The premise, asserted rather than assumed -- three separate ways this test could pass + // over a path it never entered. + val plan = CopyPlanner.plan(request.spec, request.probe) + assertEquals("the plan has to still be WebM by the time the engine sees it", Container.WEBM, plan.container) + assertNull("...and Media3 really has no muxer for it", Media3Muxers.factoryFor(plan.container)) + // Not the empty-composition refusal, which fires earlier and is a different test's subject. + assertNotEquals(VideoPlan.Drop, plan.video) + assertNotEquals(AudioPlan.Drop, plan.audio) + + val failure = try { + runCatching { + runBlocking { + withTimeout(TIMEOUT_MS) { + engine.transcode(Uri.parse("file:///dev/null"), File(context.cacheDir, "guard.webm"), request) { + } + } + } + }.exceptionOrNull() + } finally { + engine.close() + } + + assertFalse( + "the continuation was never resumed -- the refusal escaped instead of failing the job: $failure", + failure is CancellationException, + ) + // Type *and* message, and the message half is the load-bearing one. Replacing the + // requireNotNull with a fallback factory does not make the export succeed here: it lets it + // run on and fail some other way, which a bare type assertion would happily accept. + assertTrue("expected the muxer guard to refuse the job, got $failure", failure is IllegalArgumentException) + assertTrue( + "the refusal has to name the container it could not mux, got: ${failure?.message}", + failure?.message.orEmpty().contains("cannot mux") && + failure?.message.orEmpty().contains(Container.WEBM.name), + ) + } + + private companion object { + /** Nothing is decoded or muxed on this path -- the guard refuses before any of that. */ + const val TIMEOUT_MS = 10_000L + } +} diff --git a/app/src/test/java/org/libremediaconverter/work/HardwareFallbackTest.kt b/app/src/test/java/org/libremediaconverter/work/HardwareFallbackTest.kt new file mode 100644 index 0000000..9d420f9 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/work/HardwareFallbackTest.kt @@ -0,0 +1,226 @@ +package org.libremediaconverter.work + +import android.app.Application +import android.net.Uri +import androidx.media3.common.util.UnstableApi +import androidx.work.Data +import androidx.work.ListenableWorker +import androidx.work.testing.TestListenableWorkerBuilder +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.convert.ConversionDependencies +import org.libremediaconverter.convert.HardwareTranscoder +import org.libremediaconverter.convert.SoftwareTranscoder +import org.libremediaconverter.convert.installTestWorkManager +import org.libremediaconverter.model.Container +import org.libremediaconverter.model.ConversionRequest +import org.libremediaconverter.model.DeviceCodecs +import org.libremediaconverter.model.EnginePreference +import org.libremediaconverter.model.InputProbe +import org.libremediaconverter.model.OutputFormat +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import java.io.File +import java.util.UUID + +/** + * What happens when the hardware engine does not finish the job. + * + * `runMedia3OrFallBack` was eleven lines at 0% on the JVM and `isCancellation` had never been + * called by any unit test at all. Its own KDoc calls the fallback the protection against vendor + * hardware encoders that "cannot be tested for correctness", so it is the branch most likely to + * matter on a device nobody here owns — and it was reachable the whole time through + * `ConversionDependencies.hardware`, which no unit test had ever used. + * + * The sharp one is cancellation. `runMedia3OrFallBack` catches `Throwable`, so without the + * `isCancellation` re-throw a user cancelling a hardware transcode would have the app quietly + * start a *second* conversion in software — the one thing cancelling is supposed to prevent. + * + * `ForcedFailureTest` covers the failure half on a device. It does not cover the cancellation half, + * and this host cannot run it either way. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class HardwareFallbackTest { + + private lateinit var app: Application + private lateinit var hardware: RecordingHardwareTranscoder + private lateinit var software: RecordingSoftwareTranscoder + + @Before + fun setUp() { + app = RuntimeEnvironment.getApplication() + hardware = RecordingHardwareTranscoder() + software = RecordingSoftwareTranscoder() + ConversionDependencies.publisher = { AlwaysRoomPublisher(app) } + ConversionDependencies.hardware = { hardware } + ConversionDependencies.software = { software } + // A probe with real codecs, not the default: `InputProbe()` reports UNPARSEABLE, which + // PERMISSIVE.canDecode refuses, and the router would send every job here straight to + // FFmpeg without any of these tests mentioning why. + ConversionDependencies.probe = { _, _ -> H264_SOURCE } + ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE } + installTestWorkManager(app, Data.EMPTY) + } + + @After + fun tearDown() { + ConversionDependencies.reset() + } + + @Test + fun `a hardware failure runs the job again in software, on a clean staging file`() { + hardware.failWith = { error("the vendor encoder produced nothing usable") } + + val result = runBlocking { worker().doWork() } + + assertTrue("the job should still succeed, got $result", result is ListenableWorker.Result.Success) + assertEquals("the hardware engine gets exactly one attempt", 1, hardware.attempts) + assertEquals("and the job then goes to software", 1, software.attempts) + // The `staged.delete()` between the two, asserted where it is observable: FFmpeg must not + // find a half-written hardware output sitting at the path it is about to write. + assertFalse( + "the partial hardware output must be gone before FFmpeg starts", + software.outputExistedOnEntry, + ) + assertEquals("the hardware engine is closed either way", 1, hardware.closes) + } + + @Test + fun `a cancelled hardware transcode is not quietly retried in software`() { + hardware.failWith = { throw CancellationException("the user pressed Cancel") } + + assertThrows(CancellationException::class.java) { runBlocking { worker().doWork() } } + + assertEquals("the hardware engine ran", 1, hardware.attempts) + assertEquals( + "cancelling must not start a second conversion -- that is the whole point of cancelling", + 0, + software.attempts, + ) + assertEquals("and the engine is still closed on the way out", 1, hardware.closes) + } + + @Test + fun `a hardware transcode that works never reaches the software engine`() { + val result = runBlocking { worker().doWork() } + + assertTrue("got $result", result is ListenableWorker.Result.Success) + assertEquals(1, hardware.attempts) + assertEquals("the fallback is a fallback, not a second pass", 0, software.attempts) + assertEquals(1, hardware.closes) + } + + /** + * #169: the display-name fallback, which reaches further than the notification title. + * + * `inputData.getString(KEY_DISPLAY_NAME) ?: "input"` had never taken its right-hand side. The + * value is not only the foreground notification's title: it feeds `outputNameFor`, so it is + * also the filename offered in the user's save dialog. A job enqueued by an older build, or + * built by hand, carries no such key. + */ + @Test + fun `a job that names no input file still suggests an output name`() { + val result = runBlocking { worker(displayName = null).doWork() } + + assertTrue("got $result", result is ListenableWorker.Result.Success) + val suggested = (result as ListenableWorker.Result.Success) + .outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME) + assertTrue( + "expected a name built from the fallback, got $suggested", + suggested.orEmpty().startsWith("input"), + ) + } + + private fun worker(displayName: String? = DISPLAY_NAME): ConversionWorker { + val spec = OutputFormat.MP4_H265.spec + val entries = buildMap { + put(ConversionWorker.KEY_INPUT_URI, INPUT.toString()) + displayName?.let { put(ConversionWorker.KEY_DISPLAY_NAME, it) } + put(ConversionWorker.KEY_SIZE_BYTES, INPUT_BYTES) + put(ConversionWorker.KEY_CONTAINER, spec.container.name) + put(ConversionWorker.KEY_VIDEO_CODEC, spec.videoCodec.name) + put(ConversionWorker.KEY_AUDIO_CODEC, spec.audioCodec.name) + // AUTO rather than FORCE_SOFTWARE, which is what every other worker test uses and is + // exactly why this path had no coverage: forcing software never enters the function. + put(ConversionWorker.KEY_ENGINE_PREFERENCE, EnginePreference.AUTO.name) + } + return TestListenableWorkerBuilder( + context = app, + inputData = Data.Builder().putAll(entries).build(), + runAttemptCount = 0, + ).setId(JOB_ID).build() + } + + private companion object { + val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4") + const val DISPLAY_NAME = "holiday.mp4" + const val INPUT_BYTES = 1024L + val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000009") + val H264_SOURCE = InputProbe( + videoCodec = "h264", + audioCodec = "aac", + container = Container.MP4, + durationMs = 1_000, + ) + } +} + +/** + * A hardware engine that writes something before it fails, and remembers being closed. + * + * Writing first is the point, exactly as it is for `PartialThenFailingTranscoder`: an engine that + * only threw would let a missing `staged.delete()` pass unnoticed. + */ +@UnstableApi +private class RecordingHardwareTranscoder : HardwareTranscoder { + + var attempts = 0 + var closes = 0 + var failWith: (() -> Unit)? = null + + override suspend fun transcode(input: Uri, output: File, request: ConversionRequest, onProgress: (Int) -> Unit) { + attempts++ + output.writeBytes(ByteArray(PARTIAL_BYTES)) + failWith?.invoke() + } + + override fun close() { + closes++ + } + + private companion object { + const val PARTIAL_BYTES = 2048 + } +} + +/** The software engine, recording whether the hardware attempt's leftovers were cleared first. */ +private class RecordingSoftwareTranscoder : SoftwareTranscoder { + + var attempts = 0 + var outputExistedOnEntry = false + + override suspend fun run( + request: ConversionRequest, + inputPath: String, + output: File, + durationMs: Long, + onProgress: (Int) -> Unit, + ) { + attempts++ + outputExistedOnEntry = output.exists() + output.writeBytes(ByteArray(OUTPUT_BYTES)) + } + + private companion object { + const val OUTPUT_BYTES = 512 + } +}