Cover the join path with device tests

The join flow was implemented but had never run end to end: unit tests covered the planner and the argument shapes, neither of which can tell you what FFmpeg actually does with real files. Three fixtures make the strategy decision testable. clip_a and clip_b match in codec, resolution and frame rate; clip_c deliberately differs in both resolution and frame rate. Without a genuinely mismatched input there is no way to prove the re-encode branch is ever taken. The tests assert which strategy ran, not merely that output appeared. That distinction is the whole point here: the concat demuxer does not reliably reject mismatched inputs, so a naive implementation produces a file whose later segments are garbled while still exiting successfully. Each test also checks the output is long enough to contain both inputs, since a truncated join is exactly what a wrong stream copy looks like. Also covered: the list file is cleaned up, fewer than two inputs is refused, the probe distinguishes the clips the planner depends on, and the chosen strategy reaches the UI through WorkManager -- it is what tells the user whether their files were copied losslessly or re-encoded. Measured on an API 37 emulator, the two paths differ by roughly thirty times on the same pair of clips: 0.026s to stream copy against 0.829s to re-encode. That gap is itself evidence the planner is not quietly re-encoding everything. 25 instrumented tests now pass, up from 17. 65 unit tests unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-20 09:12:12 -05:00
parent e5f274183c
commit 64e146ce5d
5 changed files with 286 additions and 0 deletions
@@ -0,0 +1,177 @@
+package dev.jasonmross.mediaconverter.join
+
+import android.media.MediaExtractor
+import android.media.MediaFormat
+import android.net.Uri
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import dev.jasonmross.mediaconverter.convert.MediaProbe
+import dev.jasonmross.mediaconverter.ffmpeg.ConcatEngine
+import dev.jasonmross.mediaconverter.model.ConcatStrategy
+import kotlinx.coroutines.runBlocking
+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 join path, end to end on a device.
+ *
+ * The point of these tests is the strategy decision, not merely that a file appears.
+ * FFmpeg's `concat` demuxer does not reliably reject mismatched inputs -- it can emit a
+ * file whose later segments are garbled -- so "it produced output" is not evidence of
+ * correctness. Each test therefore checks which strategy ran *and* that the result is
+ * long enough to contain both inputs.
+ */
+@RunWith(AndroidJUnit4::class)
+class ConcatEngineTest {
+
+    private val context = InstrumentationRegistry.getInstrumentation().targetContext
+    private val engine = ConcatEngine(context)
+    private val staged = mutableListOf<File>()
+
+    private lateinit var clipA: File
+    private lateinit var clipB: File
+    private lateinit var clipMismatched: File
+
+    @Before
+    fun setUp() {
+        clipA = copyAsset("clip_a.mp4")
+        clipB = copyAsset("clip_b.mp4")
+        clipMismatched = copyAsset("clip_c_mismatched.mp4")
+    }
+
+    @After
+    fun tearDown() {
+        (staged + listOf(clipA, clipB, clipMismatched)).forEach { it.delete() }
+    }
+
+    private fun copyAsset(name: String): File {
+        val out = File(context.cacheDir, name)
+        InstrumentationRegistry.getInstrumentation().context.assets
+            .open(name)
+            .use { asset -> out.outputStream().use { asset.copyTo(it) } }
+        return out
+    }
+
+    private fun output(name: String) =
+        File(context.cacheDir, name).also { it.delete(); staged += it }
+
+    private fun durationMs(file: File): Long {
+        val extractor = MediaExtractor()
+        return try {
+            extractor.setDataSource(file.absolutePath)
+            (0 until extractor.trackCount)
+                .map { extractor.getTrackFormat(it) }
+                .filter { it.containsKey(MediaFormat.KEY_DURATION) }
+                .maxOfOrNull { it.getLong(MediaFormat.KEY_DURATION) / 1000 } ?: 0L
+        } finally {
+            extractor.release()
+        }
+    }
+
+    // --- the fast path ------------------------------------------------------
+
+    @Test
+    fun matchingClipsAreJoinedByStreamCopy() = runBlocking {
+        val out = output("joined_matching.mp4")
+        val result = engine.join(listOf(Uri.fromFile(clipA), Uri.fromFile(clipB)), out)
+
+        assertEquals(
+            "identical inputs should not need re-encoding",
+            ConcatStrategy.STREAM_COPY,
+            result.strategy,
+        )
+        assertTrue("no output produced", out.exists() && out.length() > 0)
+        // Both 2 s inputs must be present, not just the first.
+        assertTrue(
+            "joined duration ${durationMs(out)}ms is too short to hold both clips",
+            durationMs(out) >= 3_500,
+        )
+    }
+
+    // --- the correctness path ----------------------------------------------
+
+    @Test
+    fun mismatchedClipsAreReEncodedRatherThanStreamCopied() = runBlocking {
+        val out = output("joined_mismatched.mp4")
+        val result = engine.join(listOf(Uri.fromFile(clipA), Uri.fromFile(clipMismatched)), out)
+
+        // This is the case a naive implementation gets wrong: the demuxer would accept
+        // these and produce a corrupt second half.
+        assertEquals(
+            "differing resolution must force a re-encode",
+            ConcatStrategy.REENCODE,
+            result.strategy,
+        )
+        assertTrue("no output produced", out.exists() && out.length() > 0)
+        assertTrue(
+            "joined duration ${durationMs(out)}ms is too short to hold both clips",
+            durationMs(out) >= 3_500,
+        )
+    }
+
+    @Test
+    fun reEncodedOutputIsPlayableAndCarriesBothTracks() = runBlocking {
+        val out = output("joined_playable.mp4")
+        engine.join(listOf(Uri.fromFile(clipA), Uri.fromFile(clipMismatched)), out)
+
+        val extractor = MediaExtractor()
+        try {
+            extractor.setDataSource(out.absolutePath)
+            val mimes = (0 until extractor.trackCount).map {
+                extractor.getTrackFormat(it).getString(MediaFormat.KEY_MIME).orEmpty()
+            }
+            assertTrue("no video track in $mimes", mimes.any { it.startsWith("video/") })
+            assertTrue("no audio track in $mimes", mimes.any { it.startsWith("audio/") })
+        } finally {
+            extractor.release()
+        }
+    }
+
+    // --- guards -------------------------------------------------------------
+
+    @Test
+    fun joiningRefusesFewerThanTwoInputs() {
+        val out = output("joined_single.mp4")
+        val failure = runCatching {
+            runBlocking { engine.join(listOf(Uri.fromFile(clipA)), out) }
+        }.exceptionOrNull()
+        assertTrue(
+            "expected an IllegalArgumentException, got $failure",
+            failure is IllegalArgumentException,
+        )
+    }
+
+    @Test
+    fun theListFileIsCleanedUpAfterJoining() = runBlocking {
+        val out = output("joined_cleanup.mp4")
+        engine.join(listOf(Uri.fromFile(clipA), Uri.fromFile(clipB)), out)
+        assertTrue(
+            "the concat list file was left behind",
+            !File(out.parentFile, "concat_list.txt").exists(),
+        )
+    }
+
+    // --- the probe the planner depends on ----------------------------------
+
+    @Test
+    fun probeReadsThePropertiesTheStrategyDependsOn() {
+        val a = MediaProbe.probeForConcat(context, Uri.fromFile(clipA))
+        val mismatched = MediaProbe.probeForConcat(context, Uri.fromFile(clipMismatched))
+
+        assertEquals("h264", a.videoCodec)
+        assertEquals(320, a.width)
+        assertEquals(240, a.height)
+
+        assertEquals(640, mismatched.width)
+        assertEquals(480, mismatched.height)
+        assertTrue(
+            "the probe must actually distinguish these clips, or the planner cannot",
+            a.width != mismatched.width || a.height != mismatched.height,
+        )
+    }
+}
@@ -0,0 +1,109 @@
+package dev.jasonmross.mediaconverter.join
+
+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.WorkInfo
+import androidx.work.WorkManager
+import dev.jasonmross.mediaconverter.model.ConcatStrategy
+import dev.jasonmross.mediaconverter.work.ConcatWorker
+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
+
+/**
+ * Joining as background work.
+ *
+ * Covers what the engine test cannot: the WorkManager round trip, the foreground
+ * service, and the fact that the chosen strategy is reported back to the UI rather than
+ * lost inside the worker.
+ */
+@UnstableApi
+@RunWith(AndroidJUnit4::class)
+class ConcatWorkerTest {
+
+    private val context = InstrumentationRegistry.getInstrumentation().targetContext
+    private val workManager = WorkManager.getInstance(context)
+    private lateinit var clipA: File
+    private lateinit var clipB: File
+
+    @Before
+    fun setUp() {
+        clipA = copyAsset("clip_a.mp4")
+        clipB = copyAsset("clip_b.mp4")
+    }
+
+    @After
+    fun tearDown() {
+        clipA.delete()
+        clipB.delete()
+        File(context.cacheDir, "conversions").listFiles()?.forEach { it.delete() }
+    }
+
+    private fun copyAsset(name: String): File {
+        val out = File(context.cacheDir, name)
+        InstrumentationRegistry.getInstrumentation().context.assets
+            .open(name)
+            .use { asset -> out.outputStream().use { asset.copyTo(it) } }
+        return out
+    }
+
+    @Test
+    fun joinsTwoFilesAndReportsTheStrategyUsed() = runBlocking {
+        val request = ConcatWorker.request(
+            inputs = listOf(Uri.fromFile(clipA), Uri.fromFile(clipB)),
+            totalBytes = clipA.length() + clipB.length(),
+        )
+        workManager.enqueue(request).result.get()
+
+        val terminal = withTimeout(TIMEOUT_MS) {
+            workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
+        }
+
+        assertEquals(
+            "join did not succeed: ${terminal?.outputData?.getString(ConcatWorker.KEY_ERROR)}",
+            WorkInfo.State.SUCCEEDED,
+            terminal?.state,
+        )
+        // The strategy has to reach the UI: it is what tells the user whether their
+        // files were re-encoded or copied losslessly.
+        assertEquals(
+            ConcatStrategy.STREAM_COPY.name,
+            terminal?.outputData?.getString(ConcatWorker.KEY_STRATEGY),
+        )
+        val out = File(terminal!!.outputData.getString(ConcatWorker.KEY_OUTPUT_PATH)!!)
+        assertTrue("no joined file written", out.exists() && out.length() > 0)
+    }
+
+    @Test
+    fun aSingleInputFailsWithAnActionableMessage() = runBlocking {
+        val request = ConcatWorker.request(
+            inputs = listOf(Uri.fromFile(clipA)),
+            totalBytes = clipA.length(),
+        )
+        workManager.enqueue(request).result.get()
+
+        val terminal = withTimeout(TIMEOUT_MS) {
+            workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
+        }
+
+        assertEquals(WorkInfo.State.FAILED, terminal?.state)
+        val message = terminal?.outputData?.getString(ConcatWorker.KEY_ERROR).orEmpty()
+        assertTrue(
+            "the message should tell the user what to do, was: '$message'",
+            message.contains("two", ignoreCase = true),
+        )
+    }
+
+    private companion object {
+        const val TIMEOUT_MS = 180_000L
+    }
+}