Render the container row for a video nothing could name (#199)

ConverterScreen.kt:668's null arm -- DetailRow("Container", probe.container?.label ?:
"Unknown") in the VIDEO branch -- had never rendered. Every video case in FileCardTest uses
VIDEO_PROBE, which carries container = MP4.

The argument for adding it is the asymmetry, not the coverage. FileCard renders that exact
expression twice, once in AUDIO_ONLY (:660) and once in VIDEO (:668), and "an audio-only
file nothing else could describe degrades one row at a time" drives only the first. Same
expression, same fallback, one kind covered and one not -- which is the same argument
CLAUDE.md records for including ContainerCapabilities:94.

Nor is null an edge case here. InputProbe.container's own KDoc says MediaExtractor cannot
report a container at all, so it comes from FFprobe alone: any run where FFprobe did not
answer produces exactly this shape -- real codec, real dimensions, real duration, no
container. An empty value in its place would read as a rendering bug rather than as a
probe that got half its sources.

The other three rows are asserted alongside, which is what keeps this from being a copy of
the audio-only case. There, everything is unknown at once; here one field is missing from
a probe that is otherwise complete, and the rest have to be unaffected by it.

Mutation: `?: "Unknown"` -> `?: ""` at :668 only, run and restored. The AUDIO_ONLY twin at
:660 is a separate expression, and mutating that one would redden the existing test instead
-- which would prove nothing about this one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

app/src/test/java/org/libremediaconverter/convert/FileCardTest.kt

@@ -205,6 +205,34 @@ class FileCardTest {
         assertNoRow("Length")
     }
 
+    /**
+     * A video the app knows a great deal about and cannot name the container of.
+     *
+     * Not an edge case. `InputProbe.container`'s own KDoc says `MediaExtractor` cannot report a
+     * container at all -- it comes from FFprobe -- so any run where FFprobe did not answer produces
+     * exactly this: real codec, real dimensions, real duration, `container = null`.
+     *
+     * **The twin was already tested and this one was not**, which is the argument for adding it.
+     * `FileCard` renders `probe.container?.label ?: "Unknown"` twice, once in the `AUDIO_ONLY`
+     * branch (`ConverterScreen.kt:660`) and once in the `VIDEO` branch (`:668`), and
+     * `an audio-only file nothing else could describe degrades one row at a time` drives only the
+     * first. Same expression, same fallback, one kind covered. That asymmetry is the same one
+     * `CLAUDE.md` records for including `ContainerCapabilities:94`.
+     *
+     * The other rows are asserted alongside so this is not a copy of the audio-only case: there,
+     * everything is unknown at once; here, one field is missing from a probe that is otherwise
+     * complete, and the rest must be unaffected by it.
+     */
+    @Test
+    fun `a video file whose container nothing identified says so and keeps its other rows`() {
+        setFileCard(input(probe = VIDEO_PROBE.copy(container = null)))
+
+        assertRow("Container", "Unknown")
+        assertRow("Video", "${VideoCodec.H264.label} · 1920×1080")
+        assertRow("Audio", AudioCodec.AAC.label)
+        assertRow("Length", "1:30")
+    }
+
     /**
      * The row is one node, not a label node beside a value node. A test matching on `"Container"`
      * alone would pass against either shape.

app/src/test/java/org/libremediaconverter/convert/MessagelessFailureTest.kt

@@ -0,0 +1,213 @@
+package org.libremediaconverter.convert
+
+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 androidx.work.workDataOf
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.runBlocking
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.join.JoinState
+import org.libremediaconverter.join.JoinViewModel
+import org.libremediaconverter.model.ConversionRequest
+import org.libremediaconverter.model.EnginePreference
+import org.libremediaconverter.model.InputProbe
+import org.libremediaconverter.model.OutputFormat
+import org.libremediaconverter.work.ConcatWorker
+import org.libremediaconverter.work.ConversionWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+import java.util.UUID
+
+/**
+ * A failure that says nothing still has to say something.
+ *
+ * Three sites, all `ci == 0` before this file, and all the same rule:
+ *
+ * ```
+ * work/ConversionWorker.kt:316     cause.message ?: GENERIC_FAILURE_MESSAGE
+ * convert/ConversionViewModel.kt:631   e.message ?: SAVE_FAILED_MESSAGE
+ * join/JoinViewModel.kt:416           e.message ?: SAVE_FAILED_MESSAGE
+ * ```
+ *
+ * Every existing test throws *with* a message, so the right-hand side had never been evaluated
+ * anywhere in the suite. A `Throwable` carrying none is not exotic — `RuntimeException()`,
+ * `IOException()` and most platform exceptions raised without an argument all have a null message.
+ *
+ * ## Held in one class, against the ticket's suggestion
+ *
+ * #193 proposed putting each case beside the behaviour it neighbours. They are together instead,
+ * because they are one rule at three layers and because the trap below has to be explained once
+ * rather than three times. `FailedSaveRetryTest` sets the precedent for both ViewModels in one
+ * file; this extends it by one worker.
+ *
+ * ## The trap, which is why the worker case asserts what it does
+ *
+ * `ConversionStateMappingTest`'s *"a failure with nothing said still says something"* looks like it
+ * already covers the worker site. It does not: it drives the **read** side, `map(FAILED, Data.EMPTY)`,
+ * and that side has a fallback of its own (`ConversionViewModel.kt:147-149`):
+ *
+ * ```kotlin
+ * update.outputData.getString(ConversionWorker.KEY_ERROR)
+ *     ?.takeIf { it.isNotBlank() }
+ *     ?: ConversionWorker.GENERIC_FAILURE_MESSAGE
+ * ```
+ *
+ * So mutating the worker's fallback to `.orEmpty()` writes `KEY_ERROR to ""`, and the ViewModel
+ * turns that straight back into the same constant. **A test asserting on the resulting `Failed`
+ * state stays green under the mutation**, which is most likely why the write-side fallback survived
+ * three waves of test work. The worker case therefore reads `KEY_ERROR` off the worker's own
+ * `Result`, before anything downstream can repair it.
+ *
+ * The two save cases have no such second line: both write `_state.value` directly, so the state is
+ * the right thing to assert there.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class MessagelessFailureTest {
+
+    private lateinit var app: Application
+    private lateinit var publisher: RecordingPublisher
+    private lateinit var staged: File
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        publisher = RecordingPublisher(app)
+        ConversionDependencies.publisher = { publisher }
+        ConversionDependencies.probe = { _, _ -> InputProbe() }
+        staged = publisher.createStagingFile("holiday.mp4").apply { writeBytes(ByteArray(4096)) }
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    /**
+     * The engine gives up without saying why, which is what a native crash looks like from here.
+     *
+     * Asserted on the worker's own output `Data` rather than on a screen — see the class KDoc.
+     */
+    @Test
+    fun `a conversion that fails without a message still reports one`() {
+        installTestWorkManager(app, Data.EMPTY)
+        ConversionDependencies.software = { MessagelessTranscoder }
+
+        val result = runBlocking { failingWorker().doWork() }
+
+        assertTrue("the job must fail rather than retry, got $result", result is ListenableWorker.Result.Failure)
+        assertEquals(
+            "a failure with no message must still put something on screen",
+            ConversionWorker.GENERIC_FAILURE_MESSAGE,
+            (result as ListenableWorker.Result.Failure).outputData.getString(ConversionWorker.KEY_ERROR),
+        )
+    }
+
+    @Test
+    fun `a save that fails without a message still reports one`() {
+        installTestWorkManager(app, conversionOutput())
+        val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
+        viewModel.onInputPicked(Uri.parse("content://test/holiday.mkv"))
+        awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
+        viewModel.convert()
+        awaitState(viewModel.state, "Converted") { it is ConversionState.Converted }
+
+        publisher.publishFailure = RuntimeException()
+        viewModel.save(DESTINATION)
+
+        val failed = awaitState(viewModel.state, "Failed") { it is ConversionState.Failed } as ConversionState.Failed
+        assertEquals(SAVE_FAILED_MESSAGE, failed.message)
+        // The handle travels even on the wordless path. Without this, a fallback that also dropped
+        // `pending` would pass -- and the file would be unreachable from the screen that just said
+        // the save failed.
+        assertNotNull("a wordless failure must still offer the file again", failed.retry)
+    }
+
+    @Test
+    fun `a join save that fails without a message still reports one`() {
+        installTestWorkManager(app, joinOutput())
+        val viewModel = JoinViewModel(app, Dispatchers.Unconfined)
+        viewModel.onInputsPicked(listOf(Uri.parse("content://test/a.mp4"), Uri.parse("content://test/b.mp4")))
+        awaitState(viewModel.state, "Ready") { it is JoinState.Ready }
+        viewModel.join()
+        awaitState(viewModel.state, "Joined") { it is JoinState.Joined }
+
+        publisher.publishFailure = RuntimeException()
+        viewModel.save(DESTINATION)
+
+        val failed = awaitState(viewModel.state, "Failed") { it is JoinState.Failed } as JoinState.Failed
+        assertEquals(SAVE_FAILED_MESSAGE, failed.message)
+        assertNotNull("a wordless failure must still offer the file again", failed.retry)
+    }
+
+    /**
+     * `FORCE_SOFTWARE` so the failure comes straight out of `runFFmpeg`.
+     *
+     * `AUTO` would enter `runMedia3OrFallBack`, whose catch runs the job a second time in software
+     * — the same exception would arrive, but through a path this test is not about and which
+     * `HardwareFallbackTest` already owns.
+     */
+    private fun failingWorker(): ConversionWorker {
+        val spec = OutputFormat.MP4_H265.spec
+        return TestListenableWorkerBuilder<ConversionWorker>(
+            context = app,
+            inputData = workDataOf(
+                ConversionWorker.KEY_INPUT_URI to "file:///tmp/holiday.mp4",
+                ConversionWorker.KEY_DISPLAY_NAME to "holiday.mp4",
+                ConversionWorker.KEY_CONTAINER to spec.container.name,
+                ConversionWorker.KEY_VIDEO_CODEC to spec.videoCodec.name,
+                ConversionWorker.KEY_AUDIO_CODEC to spec.audioCodec.name,
+                ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name,
+            ),
+            runAttemptCount = 0,
+        ).setId(JOB_ID).build()
+    }
+
+    private fun conversionOutput() = workDataOf(
+        ConversionWorker.KEY_OUTPUT_PATH to staged.absolutePath,
+        ConversionWorker.KEY_SUGGESTED_NAME to SUGGESTED_NAME,
+        ConversionWorker.KEY_MIME_TYPE to JOB_MIME_TYPE,
+    )
+
+    private fun joinOutput() = workDataOf(
+        ConcatWorker.KEY_OUTPUT_PATH to staged.absolutePath,
+        ConcatWorker.KEY_SUGGESTED_NAME to SUGGESTED_NAME,
+        ConcatWorker.KEY_MIME_TYPE to JOB_MIME_TYPE,
+    )
+
+    private companion object {
+        val DESTINATION: Uri = Uri.parse("content://test/destination.mp4")
+        val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000019a")
+        const val SUGGESTED_NAME = "holiday.mp4"
+        const val JOB_MIME_TYPE = "video/mp4"
+    }
+}
+
+/**
+ * An engine that gives up without saying why.
+ *
+ * `RuntimeException()` rather than a subclass with a blank message: `Throwable.message` is *null*
+ * here, which is the case the elvis exists for. A blank-but-present message takes the left-hand
+ * side and is a different path — `ConversionStateMappingTest` covers that one, on the read side.
+ */
+@UnstableApi
+private object MessagelessTranscoder : SoftwareTranscoder {
+    override suspend fun run(
+        request: ConversionRequest,
+        inputPath: String,
+        output: File,
+        durationMs: Long,
+        onProgress: (Int) -> Unit,
+    ): Unit = throw RuntimeException()
+}

app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt

@@ -190,6 +190,30 @@ class FFmpegCommandBuilderTest {
         assertPair(cmd(OutputFormat.M4A_AAC), "-b:a", "192k")
     }
 
+    /**
+     * Turning audio off, which the Advanced picker offers and nothing had ever built a command for.
+     *
+     * `audioArgs`' `Drop` arm was `ci == 0`. The suite's only `-an` assertion is in
+     * `gif generates a palette to avoid banding and drops audio`, and that one comes from the image
+     * path (`FFmpegCommandBuilder.kt:79`/`:90`), which emits `-an` directly and never reaches
+     * `audioArgs`. Two sites, one string, one tested.
+     *
+     * It is a live path rather than defensive code: `AdvancedPicker` renders all of
+     * `AudioCodec.entries` including `NONE`, `ContainerCapabilities.validate` permits audio-off
+     * whenever the input has video, and MKV routes the job to FFmpeg.
+     *
+     * Both halves are asserted. `-an` alone would still pass if the arm fell through to the `else`
+     * and emitted an AAC encoder beside it -- a file that is silent because the flag won, carrying
+     * an encoder nobody asked for.
+     */
+    @Test
+    fun `turning audio off drops the track instead of encoding one`() {
+        val args = cmd(OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.NONE))
+
+        assertTrue("audio turned off must emit -an, got $args", args.contains("-an"))
+        assertFalse("a dropped track must not also carry an encoder, got $args", args.contains("-c:a"))
+    }
+
     @Test
     fun `audio only formats never carry a video encoder`() {
         listOf(OutputFormat.MP3, OutputFormat.FLAC, OutputFormat.WAV, OutputFormat.OPUS)