Merge branch 'main' into test/r38-2-filecard

Hold the Advanced panel's gate, and the error card outside it

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

@@ -0,0 +1,152 @@
+package org.libremediaconverter.convert
+
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.libremediaconverter.model.AudioCodec
+import org.libremediaconverter.model.Container
+import org.libremediaconverter.model.EnginePreference
+import org.libremediaconverter.model.OutputSpec
+import org.libremediaconverter.model.VideoCodec
+
+/**
+ * The four pure helpers behind the converter screen's prose, pinned at the points where they
+ * change what they say.
+ *
+ * No Compose rule and no Robolectric: these are `String` in, `String` out, and running them under a
+ * device sandbox would buy nothing while hiding the boundaries in a rendered tree.
+ *
+ * The defect each group bites on:
+ *
+ * - **[formatBytes] picks a unit by comparing against three thresholds.** Every one of them is a
+ *   `>=`, and a `>` would move a file sitting exactly on a boundary into the unit below -- `1 GB`
+ *   shown as `1000.0 MB`. Only a value *on* the threshold can tell the two apart, so each of the
+ *   three is asserted at the boundary and one below it. The unit prefixes are decimal, matching
+ *   what the file manager and the provider report, not powers of two.
+ * - **[formatDuration] has no hours field.** An hour-long recording reads `60:00`, and that is the
+ *   contract rather than an oversight -- the row is a length, not a clock. Pinned so that adding
+ *   hours is a deliberate change with a red test in front of it instead of a silent reformat.
+ * - **[describe] builds the suggestion-chip label out of up to three parts**, and the parts are
+ *   conditional: [VideoCodec.NONE] and [AudioCodec.NONE] drop out entirely, so an image output
+ *   with neither track has to render as the container alone rather than as a container followed
+ *   by a dangling separator.
+ * - **[EnginePreference] carries no `label` property**, unlike every other enum the screen
+ *   renders; its three display strings live in a `when` in the screen file. Adding a constant is
+ *   caught by the compiler because that `when` is exhaustive, but nothing stops two constants
+ *   being given the same string, which is what the distinctness assertion is for.
+ */
+class ConverterFormattersTest {
+
+    @Test
+    fun `bytes below a kilobyte are counted exactly`() {
+        assertEquals("0 B", formatBytes(0))
+        assertEquals("1 B", formatBytes(1))
+        assertEquals("999 B", formatBytes(999))
+    }
+
+    @Test
+    fun `each unit starts exactly on its threshold rather than one byte past it`() {
+        assertEquals("1 kB", formatBytes(1_000))
+        assertEquals("1.0 MB", formatBytes(1_000_000))
+        assertEquals("1.0 GB", formatBytes(1_000_000_000))
+    }
+
+    /**
+     * One byte below each threshold, which is the half a `>=` to `>` change leaves alone. Both
+     * halves are needed: the boundary values alone would still pass if the comparison let
+     * everything through.
+     */
+    @Test
+    fun `a value just below a threshold stays in the smaller unit`() {
+        assertEquals("999 B", formatBytes(999))
+        assertEquals("1000 kB", formatBytes(999_999))
+        assertEquals("1000.0 MB", formatBytes(999_999_999))
+    }
+
+    @Test
+    fun `a real file size reads as one decimal place`() {
+        assertEquals("12.3 MB", formatBytes(12_345_678))
+        assertEquals("1.5 GB", formatBytes(1_500_000_000))
+    }
+
+    @Test
+    fun `a duration is minutes and zero-padded seconds`() {
+        assertEquals("0:00", formatDuration(0))
+        assertEquals("0:01", formatDuration(1_000))
+        assertEquals("0:59", formatDuration(59_000))
+        assertEquals("1:00", formatDuration(60_000))
+        assertEquals("1:30", formatDuration(90_000))
+    }
+
+    /** Sub-second remainders are dropped rather than rounded up into the next second. */
+    @Test
+    fun `a partial second does not become a whole one`() {
+        assertEquals("0:00", formatDuration(999))
+        assertEquals("0:59", formatDuration(59_999))
+    }
+
+    /** No hours field, deliberately: an hour is `60:00` and two hours are `120:00`. */
+    @Test
+    fun `an hour and beyond keeps counting in minutes`() {
+        assertEquals("60:00", formatDuration(3_600_000))
+        assertEquals("61:01", formatDuration(3_661_000))
+        assertEquals("120:00", formatDuration(7_200_000))
+    }
+
+    @Test
+    fun `a spec with both tracks names the container and joins the two codecs`() {
+        assertEquals(
+            "MP4 · H.264 + AAC",
+            describe(OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC)),
+        )
+    }
+
+    @Test
+    fun `a track set to none is left out instead of being named none`() {
+        assertEquals(
+            "MP3 · MP3",
+            describe(OutputSpec(Container.MP3, VideoCodec.NONE, AudioCodec.MP3)),
+        )
+        assertEquals(
+            "MP4 · H.264",
+            describe(OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.NONE)),
+        )
+    }
+
+    /** An image output has neither track, so there is nothing for the separator to separate. */
+    @Test
+    fun `a spec with no tracks at all is the container alone, with no trailing separator`() {
+        assertEquals("GIF", describe(OutputSpec(Container.GIF, VideoCodec.NONE, AudioCodec.NONE)))
+        assertEquals(
+            "PNG frames",
+            describe(OutputSpec(Container.IMAGE_SEQUENCE, VideoCodec.NONE, AudioCodec.NONE)),
+        )
+    }
+
+    /** `Copy` is a codec here, not the absence of one, so a remux describes both tracks. */
+    @Test
+    fun `a remux names copy on both tracks rather than dropping them`() {
+        assertEquals(
+            "Matroska · Copy + Copy",
+            describe(OutputSpec(Container.MKV, VideoCodec.COPY, AudioCodec.COPY)),
+        )
+    }
+
+    @Test
+    fun `each engine preference has the wording the chips show`() {
+        assertEquals("Automatic", EnginePreference.AUTO.label())
+        assertEquals("Prefer hardware", EnginePreference.PREFER_HARDWARE.label())
+        assertEquals("Force software", EnginePreference.FORCE_SOFTWARE.label())
+    }
+
+    /**
+     * Two constants sharing a label would render as two identical chips, one of which the user
+     * could not choose deliberately. The exhaustive `when` cannot catch that; this does.
+     */
+    @Test
+    fun `no two engine preferences render the same chip`() {
+        val labels = EnginePreference.entries.map { it.label() }
+
+        assertEquals(EnginePreference.entries.size, labels.toSet().size)
+        assertEquals(emptyList<String>(), labels.filter { it.isBlank() })
+    }
+}

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

@@ -0,0 +1,254 @@
+package org.libremediaconverter.convert
+
+import android.net.Uri
+import androidx.compose.ui.test.assertCountEquals
+import androidx.compose.ui.test.assertTextEquals
+import androidx.compose.ui.test.onChildren
+import androidx.compose.ui.test.onNodeWithTag
+import androidx.media3.common.util.UnstableApi
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.createDrainedComposeRule
+import org.libremediaconverter.model.AudioCodec
+import org.libremediaconverter.model.Container
+import org.libremediaconverter.model.InputKind
+import org.libremediaconverter.model.InputProbe
+import org.libremediaconverter.model.VideoCodec
+import org.libremediaconverter.ui.TestTags
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * What the source-info card says when it does not know something.
+ *
+ * The defect is a card that invents an answer instead of admitting it has none. Two of them are
+ * live here and neither had a test before this file:
+ *
+ * - **`InputFile.sizeBytes` is nullable and the card is the reader that has to say so in words.**
+ *   `sizeBytes` used to be `0L` for "nobody told me", and [UnknownInputSizeTest] records what that
+ *   cost at the space check. The card is the other reader, and its failure mode is the mirror
+ *   image: hand the null to `formatBytes` and it renders `"0 B"` -- a measurement, shown to the
+ *   user, that no provider ever made. It renders **independently of the probe**, which is why the
+ *   same assertion appears twice below, with the probe present and absent. That independence is
+ *   the contract; a test covering only the probed case would leave the branch a user actually hits
+ *   first -- the card is on screen before the probe finishes -- unguarded.
+ * - **The codec rows degrade in words too.** `CodecNames.describeVideo`/`describeAudio` answer
+ *   `"Unknown"` for a codec nothing named, the `VIDEO` branch answers `"No audio track"` for a file
+ *   with no audio, and the two `> 0` guards drop the dimension and length rows rather than printing
+ *   `0` and `0:00`. Each of those has a case below on **both** sides of the guard, because a test
+ *   of the present side alone stays green with the guard deleted.
+ *
+ * ### What cannot be asserted here, so that it is a decision rather than an omission
+ *
+ * The `probe == null` branch exits before `HorizontalDivider`, and **the divider's absence is not
+ * observable from a test**: Material 3 renders it as a `Box` with no semantics modifier, so it
+ * contributes no node to the semantics tree at all. What is asserted instead is everything the
+ * divider precedes -- no detail row for any label the four kind branches can emit -- plus the
+ * card's child count, which pins "these three texts and nothing else" without having to enumerate.
+ *
+ * The early exit itself is enforced by the compiler rather than by this file, which the PR body
+ * records: deleting `return@Column` un-smart-casts `probe`, and the `probe.kind` below it stops
+ * compiling. The mutation that reddens the test here is the compilable form of that regression --
+ * defaulting the null away with `?: InputProbe()` and letting the kind rows render.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class FileCardTest {
+
+    @get:Rule
+    val composeRule = createDrainedComposeRule()
+
+    @Test
+    fun `a file no provider could measure says so in words rather than showing a zero`() {
+        setFileCard(input(sizeBytes = null, probe = VIDEO_PROBE))
+
+        composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES)
+            .assertTextEquals("Size unknown")
+    }
+
+    /**
+     * The same line, with no probe at all. Separate from the case above rather than folded into
+     * it because `setContent` may only be called once per rule, and because two independent reds
+     * are the evidence that the size line does not depend on the probe.
+     */
+    @Test
+    fun `the size line says the same thing while the probe is still running`() {
+        setFileCard(input(sizeBytes = null, probe = null))
+
+        composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES)
+            .assertTextEquals("Size unknown")
+    }
+
+    @Test
+    fun `a size that was reported is formatted rather than replaced by the unknown line`() {
+        setFileCard(input(sizeBytes = 12_345_678L, probe = VIDEO_PROBE))
+
+        composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertTextEquals("clip.mkv")
+        composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES).assertTextEquals("12.3 MB")
+    }
+
+    /**
+     * The note and the emptiness are one behaviour, so they are one test: a regression that keeps
+     * the note but renders the rows anyway would leave a note-only test green.
+     */
+    @Test
+    fun `while the probe is still running the card shows the reading note and nothing else`() {
+        setFileCard(input(probe = null))
+
+        composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NOTE)
+            .assertTextEquals("Reading…")
+        assertNoDetailRows()
+        // Name, size, note. Catches a row whose label is not in EVERY_ROW_LABEL as well.
+        composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).onChildren().assertCountEquals(3)
+    }
+
+    @Test
+    fun `a file nothing could read gets the explanatory line instead of unknown codecs`() {
+        setFileCard(input(probe = InputProbe(kind = InputKind.UNPARSEABLE)))
+
+        composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NOTE)
+            .assertTextEquals("Could not identify this file. It will be converted with FFmpeg.")
+        assertNoDetailRows()
+    }
+
+    @Test
+    fun `an image gets its type and its pixel dimensions`() {
+        setFileCard(input(probe = InputProbe(kind = InputKind.IMAGE, width = 1920, height = 1080)))
+
+        assertRow("Type", "Image")
+        assertRow("Size", "1920×1080")
+    }
+
+    /** The `width > 0` guard, from the side that would print `0×0` if it were dropped. */
+    @Test
+    fun `an image whose dimensions nothing reported gets the type row alone`() {
+        setFileCard(input(probe = InputProbe(kind = InputKind.IMAGE)))
+
+        assertRow("Type", "Image")
+        assertNoRow("Size")
+    }
+
+    @Test
+    fun `an audio-only file says it has no video track rather than leaving the row blank`() {
+        setFileCard(
+            input(
+                probe = InputProbe(
+                    audioCodec = "aac",
+                    hasVideo = false,
+                    durationMs = 90_000,
+                    kind = InputKind.AUDIO_ONLY,
+                    container = Container.MP3,
+                ),
+            ),
+        )
+
+        assertRow("Container", Container.MP3.label)
+        assertRow("Video", "No video track")
+        assertRow("Audio", AudioCodec.AAC.label)
+        assertRow("Length", "1:30")
+        assertNoRow("Type")
+        assertNoRow("Size")
+    }
+
+    /**
+     * Everything the audio branch can fail to know, at once: no container, no codec name, no
+     * duration. Each degrades in its own words, and the length row disappears rather than
+     * claiming `0:00`.
+     */
+    @Test
+    fun `an audio-only file nothing else could describe degrades one row at a time`() {
+        setFileCard(input(probe = InputProbe(hasVideo = false, kind = InputKind.AUDIO_ONLY)))
+
+        assertRow("Container", "Unknown")
+        assertRow("Video", "No video track")
+        assertRow("Audio", "Unknown")
+        assertNoRow("Length")
+    }
+
+    @Test
+    fun `a video file composes its codec with its dimensions on one row`() {
+        setFileCard(input(probe = VIDEO_PROBE))
+
+        assertRow("Container", Container.MP4.label)
+        assertRow("Video", "${VideoCodec.H264.label} · 1920×1080")
+        assertRow("Audio", AudioCodec.AAC.label)
+        assertRow("Length", "1:30")
+    }
+
+    /**
+     * `"No audio track"` rather than `describeAudio(null)`'s `"Unknown"`. The video branch knows
+     * the difference between a track it could not name and a track that is not there; the audio
+     * branch above cannot, because a file with no audio is not audio-only.
+     */
+    @Test
+    fun `a video file with no audio track says so instead of naming an unknown codec`() {
+        setFileCard(input(probe = VIDEO_PROBE.copy(audioCodec = null)))
+
+        assertRow("Audio", "No audio track")
+    }
+
+    /** Both `> 0` guards on the video branch, plus the codec name nothing supplied. */
+    @Test
+    fun `a video file missing its codec, dimensions and duration omits them rather than faking them`() {
+        setFileCard(
+            input(
+                probe = VIDEO_PROBE.copy(
+                    videoCodec = null,
+                    width = 0,
+                    height = 0,
+                    durationMs = 0,
+                ),
+            ),
+        )
+
+        assertRow("Video", "Unknown")
+        assertNoRow("Length")
+    }
+
+    /**
+     * The row is one node, not a label node beside a value node. A test matching on `"Container"`
+     * alone would pass against either shape.
+     */
+    @Test
+    fun `a detail row renders its label and its value as a single node`() {
+        composeRule.setContent { DetailRow("Container", "Matroska") }
+
+        composeRule.onNodeWithTag(TestTags.Converter.detailRow("Container"))
+            .assertTextEquals("Container: Matroska")
+    }
+
+    private fun setFileCard(input: InputFile) = composeRule.setContent { FileCard(input) }
+
+    private fun input(sizeBytes: Long? = 12_345_678L, probe: InputProbe? = VIDEO_PROBE) = InputFile(
+        uri = Uri.parse("content://test/clip.mkv"),
+        displayName = "clip.mkv",
+        sizeBytes = sizeBytes,
+        probe = probe,
+    )
+
+    private fun assertRow(label: String, value: String) {
+        composeRule.onNodeWithTag(TestTags.Converter.detailRow(label))
+            .assertTextEquals("$label: $value")
+    }
+
+    private fun assertNoRow(label: String) {
+        composeRule.onNodeWithTag(TestTags.Converter.detailRow(label)).assertDoesNotExist()
+    }
+
+    private fun assertNoDetailRows() = EVERY_ROW_LABEL.forEach(::assertNoRow)
+
+    private companion object {
+        /** Every label the four kind branches can emit, so absence can be asserted exhaustively. */
+        val EVERY_ROW_LABEL = listOf("Container", "Video", "Audio", "Length", "Type", "Size")
+
+        val VIDEO_PROBE = InputProbe(
+            videoCodec = "h264",
+            audioCodec = "aac",
+            durationMs = 90_000,
+            kind = InputKind.VIDEO,
+            container = Container.MP4,
+            width = 1920,
+            height = 1080,
+        )
+    }
+}