diff --git a/app/src/test/java/org/libremediaconverter/codec/CodecVocabularyTest.kt b/app/src/test/java/org/libremediaconverter/codec/CodecVocabularyTest.kt index 7045b7f..5e9467a 100644 --- a/app/src/test/java/org/libremediaconverter/codec/CodecVocabularyTest.kt +++ b/app/src/test/java/org/libremediaconverter/codec/CodecVocabularyTest.kt @@ -7,6 +7,7 @@ import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test import org.libremediaconverter.model.CodecNames +import org.libremediaconverter.model.InputProbe import org.libremediaconverter.model.VideoCodec /** @@ -133,6 +134,38 @@ class CodecVocabularyTest { * landed, a device with no HEVC decoder answered true for `x265` and Media3 was handed a job it * could not do; now the router sends it to FFmpeg without spending the attempt. */ + /** + * The sentinel is not just another unknown name, and the difference is the whole guard. + * + * `canDecode` ends `?: true` -- a name neither table knows keeps the permissive answer, because + * the app would rather try than refuse a file it might handle. `InputProbe.UNPARSEABLE` has to + * be the exception: the platform has *already* failed to parse the input, so there is nothing + * for a decoder to be permissive about, and waving it through spends a Media3 attempt on a job + * that cannot start. + * + * The `cinepak` line is what makes the sentinel line mean something. Without it, deleting the + * early return leaves this test green -- both names would fall through to the same `?: true`. + * The pair is the assertion. + * + * `DeviceCodecs.PERMISSIVE` carries the same rule and `ConversionRouterTest` pins its routing + * consequence. This is the implementation that runs on a device. + */ + @Test + fun `the unparseable sentinel is refused even where an unknown name is waved through`() { + val everything = AndroidDeviceCodecs.forTesting( + encoders = emptySet(), + decoders = setOf("video/avc", "video/hevc"), + ) + assertFalse( + "the platform could not parse this input, so there is nothing to decode with", + everything.canDecode(InputProbe.UNPARSEABLE), + ) + assertTrue( + "a merely unknown name still keeps the permissive answer", + everything.canDecode("cinepak"), + ) + } + @Test fun `a device without the decoder now says so for the aliases it used to wave through`() { val hevcOnly = AndroidDeviceCodecs.forTesting(encoders = emptySet(), decoders = setOf("video/hevc")) diff --git a/app/src/test/java/org/libremediaconverter/convert/OutputPublisherPublishTest.kt b/app/src/test/java/org/libremediaconverter/convert/OutputPublisherPublishTest.kt index 65ec5d6..7d8e972 100644 --- a/app/src/test/java/org/libremediaconverter/convert/OutputPublisherPublishTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/OutputPublisherPublishTest.kt @@ -232,6 +232,12 @@ class OutputPublisherPublishTest { RowShape.NO_SIZE_COLUMN to "a cursor with no SIZE column", RowShape.NULL_SIZE to "a cursor whose SIZE cell is null", RowShape.NO_ROWS to "a cursor holding no rows", + // The third case the KDoc names -- "a resolver call that throws" -- and the one the + // list was missing. It reaches `?: false` through `runCatching` rather than through a + // cursor answer, so it is the only one of the four that proves the catch is load + // bearing: a provider that revokes its grant between the picker and the write must not + // have its document deleted on the way out. + RowShape.QUERY_THROWS to "a provider that throws out of query", ).forEach { (shape, description) -> FakeSafProvider.deleteRequests.clear() FakeSafProvider.backingFile(documentUri).writeBytes(ByteArray(0)) diff --git a/app/src/test/java/org/libremediaconverter/convert/UnreadableJoinInputTest.kt b/app/src/test/java/org/libremediaconverter/convert/UnreadableJoinInputTest.kt new file mode 100644 index 0000000..460a061 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/UnreadableJoinInputTest.kt @@ -0,0 +1,70 @@ +package org.libremediaconverter.convert + +import android.net.Uri +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.model.ConcatPlanner +import org.libremediaconverter.model.ConcatStrategy +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +/** + * A clip in a join that nothing could read, from the probe all the way to the strategy. + * + * Both halves of this are covered already, and separately: `MediaProbeTrackWalkTest` pins what + * `concatInputFrom` makes of a track list, and `ConcatPlannerTest`'s + * `an unknown codec is not treated as a match` pins what the planner does with a hand-built + * `ConcatInput(video = null)`. **Nothing spanned the two**, and the span is the load-bearing part: + * the planner's safety rests on the probe really producing that shape, and the hand-built fixture + * would go on passing if it stopped. + * + * Measured rather than asserted: mutating `concatInputFrom`'s initial `video` to a non-null + * placeholder leaves `ConcatPlannerTest` green and turns this red. + * + * ## The asymmetry this protects + * + * `ConcatPlanner` guards its video check against a null codec (`ConcatStrategy.kt:51`) and its + * audio check not at all (`:54`). **That is correct, not an oversight.** `MediaProbe.shortName` + * returns a non-null `String`, so in `concatInputFrom` a null `audioCodec` means the track is + * *absent* — and two clips with no audio genuinely do match. A null `videoCodec` carries both + * meanings, absent or unreadable, which is why only that one is guarded. + * + * So the audio check is safe *because* the video guard fires first on a clip nothing could read. + * Nothing wrote that coupling down and nothing held it. + * + * ## What this deliberately does not cover + * + * `probeForConcat`'s `catch` arm (`MediaProbe.kt:300-302`). It is **not reachable on the JVM**: + * Robolectric's `MediaExtractor` never throws from `setDataSource`, measured across an + * unregistered `content://` authority, a missing `file://`, a file of garbage bytes and an `http://` + * URL — all four returned normally with `trackCount = 0`. So the failure arrives here as an empty + * track list rather than as an exception, which reaches the same `ConcatInput(null, null, 0, 0, 0)` + * by the other road. The catch stays device-only, and this file does not pretend otherwise. + */ +@RunWith(RobolectricTestRunner::class) +class UnreadableJoinInputTest { + + @Test + fun `a clip nothing could read probes as unknown, and an unknown clip is re-encoded`() { + val unreadable = MediaProbe.probeForConcat(RuntimeEnvironment.getApplication(), UNREADABLE) + + assertNull("an unreadable clip proves nothing about its video codec", unreadable.videoCodec) + assertNull("nor about its audio codec", unreadable.audioCodec) + assertEquals("nor about its dimensions", 0, unreadable.width) + assertEquals(0, unreadable.height) + assertEquals(0, unreadable.frameRate) + + assertEquals( + "a clip nothing could read is not evidence of a match with anything", + ConcatStrategy.REENCODE, + ConcatPlanner.plan(listOf(unreadable, unreadable)), + ) + } + + private companion object { + /** `content://` so the probe takes the SAF branch a real pick takes. Nothing answers it. */ + val UNREADABLE: Uri = Uri.parse("content://test/vanished.mp4") + } +} diff --git a/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt b/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt index 30a44e5..a1308ea 100644 --- a/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt @@ -451,6 +451,99 @@ class ContainerCapabilitiesTest { } } + /** + * The video twin of `no audio track is accepted by every container in both modes`. + * + * Dead in production today, and deliberately so: every caller guards `NONE` before asking the + * matrix, so nothing reaches this arm through the app. **The asymmetry is the argument, not the + * reachability** -- its audio counterpart at the top of the same `when` has had a dedicated + * test since #136, and one of a matched pair being covered is how a later reader concludes the + * other was considered and exempted. It was not; it was simply missed. + * + * Not the same shape as the two `COPY -> error(...)` arms, which `docs/coverage-read-findings.md` + * records as a named exemption (F4). Those are guards that must not be provokable. This is a + * documented answer -- "no video track fits anywhere" -- and an answer is a thing to pin. + */ + @Test + fun `no video track is accepted by every container in both modes`() { + Container.entries.forEach { container -> + listOf(CodecMode.COPY, CodecMode.ENCODE).forEach { mode -> + assertTrue( + "$container should accept no video track ($mode)", + ContainerCapabilities.accepts(container, VideoCodec.NONE, mode), + ) + } + } + } + + /** + * A suggestion that keeps the codec the user asked for, rather than falling back to the + * container's first encodable one. + * + * `repairVideo`'s third arm -- "the request is not a copy, and this container can encode it" -- + * is the one that preserves intent, and it was the only arm of the four nothing reached. The + * property test above executes `repairVideo` on every case it walks and lands elsewhere each + * time: an explicit COPY that works, a source the container can carry untouched, or no video + * track at all. + * + * The route is indirect because it is the only one the app has. VP9 into WebM is a perfectly + * good video request; what makes it invalid is the *audio* -- WebM carries Opus and Vorbis, not + * AAC. So `validateAudio` refuses, `suggestions` looks for a container that can hold what was + * asked for, and MP4 can encode VP9. The suggestion has to come back carrying VP9: swapping to + * the container's first encodable codec would discard the choice the user made. + */ + @Test + fun `a repaired suggestion keeps the video codec the user chose`() { + val invalid = ContainerCapabilities.validate( + OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.AAC), + h264Source, + ) + + assertTrue("WebM cannot hold AAC, so this spec is invalid", invalid is Validation.Invalid) + val suggestions = (invalid as Validation.Invalid).suggestions + assertTrue( + "expected a suggestion that still encodes VP9, got $suggestions", + suggestions.any { it.videoCodec == VideoCodec.VP9 }, + ) + assertEverySuggestionValid(invalid, h264Source) + } + + /** + * The fallback in `firstContainerHolding`: when the input's own container cannot hold the + * codec the user asked for, any container that can will do. + * + * The preferred half -- "the container the input already uses" -- is what every other case + * reaches, because they all start from a file whose own container carries the codec in + * question. The elvis after it had never run. + * + * AVI is the input that makes it run: AVI predates H.265 and has no mapping for it, so asking + * an AVI for H.265 is refused, and the container the input already uses cannot be part of the + * answer. Without the fallback the only candidates left are AVI itself and the container + * holding the *source* codec -- also AVI -- so the refusal still offers something, but what it + * offers is H.264: the app quietly declines the codec the user asked for instead of moving them + * to a container that supports it. + * + * That is why this asserts the codec survives rather than that the list is non-empty. A + * non-empty assertion passes with the fallback deleted -- measured, not assumed. + */ + @Test + fun `an input whose container cannot hold the requested codec is moved, not downgraded`() { + val aviSource = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.AVI) + + val invalid = ContainerCapabilities.validate( + OutputSpec(Container.AVI, VideoCodec.H265, AudioCodec.AAC), + aviSource, + ) + + assertTrue("AVI has no mapping for H.265", invalid is Validation.Invalid) + val suggestions = (invalid as Validation.Invalid).suggestions + assertTrue( + "expected a container that can actually hold H.265, got $suggestions", + suggestions.any { it.videoCodec == VideoCodec.H265 }, + ) + assertEverySuggestionValid(invalid, aviSource) + } + @Test fun `resolving audio COPY before asking the matrix is required`() { // The audio twin of `resolving COPY before asking the matrix is required`, and the reason is diff --git a/app/src/test/java/org/libremediaconverter/work/JobSnapshotsTest.kt b/app/src/test/java/org/libremediaconverter/work/JobSnapshotsTest.kt index 4af2fda..78af61f 100644 --- a/app/src/test/java/org/libremediaconverter/work/JobSnapshotsTest.kt +++ b/app/src/test/java/org/libremediaconverter/work/JobSnapshotsTest.kt @@ -16,6 +16,7 @@ import androidx.work.testing.WorkManagerTestInitHelper import androidx.work.workDataOf import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -125,6 +126,39 @@ class JobSnapshotsTest { assertEquals(newer.absolutePath, Reattachment.choose(snapshots)?.job?.outputPath) } + /** + * A job in the tag query that never recorded an output path at all. + * + * Distinct from the three cases above, which all *have* a path and differ in what it names. A + * job still running, or one that finished without writing its result key, carries no path at + * all -- and `getWorkInfosByTagFlow` returns it alongside the finished ones, because the tag is + * the worker class and every attempt ever enqueued carries it. + * + * The guard is the `?.` in `path?.let(::File)`. Without it the null goes straight into a `File` + * constructor. What this pins is the consequence rather than the null check: such a job must + * not be offered as a result, so `Reattachment.choose` has to walk past it to the job that + * really produced a file. Choosing it would put a Converted screen in front of the user with a + * Save button that has nothing to save. + */ + @Test + fun `a job that recorded no output path is not offered as a result`() { + val real = stagedFile("real.mp4", bytes = 4096) + finishedWithOutput(real) + finishedWithNoOutput() + + val snapshots = snapshots() + + assertEquals("both jobs carry the tag, so both come back", 2, snapshots.size) + val silent = snapshots.single { it.outputPath == null } + assertFalse("no path means no output, not an empty one", silent.outputExists) + assertEquals("and no time either, for the same reason", 0L, silent.outputModifiedAt) + assertEquals( + "the reattachment has to walk past it to the job that really produced a file", + real.absolutePath, + Reattachment.choose(snapshots)?.job?.outputPath, + ) + } + private fun snapshots(): List = runBlocking { workManager.jobSnapshots( tag = ConversionWorker::class.java.name, @@ -152,6 +186,11 @@ class JobSnapshotsTest { ).result.get() } + /** A job that carries the tag and no result key -- still running, or finished without one. */ + private fun finishedWithNoOutput() { + workManager.enqueue(OneTimeWorkRequestBuilder().build()).result.get() + } + private companion object { /** Two fixed moments a day apart, so the ordering is stated rather than raced for. */ const val OLDER_MS = 1_700_000_000_000L