From 39327beea72985c1386ca492a1e2899dc3bafa96 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Sat, 5 Sep 2026 21:13:06 -0500 Subject: [PATCH 1/2] Assert the join failure message against a real FFmpeg session #217 closed by noting the join legs had not been run locally. Running them would not have answered it: nothing on either source set drove a real join *failure*, so the message unified in #203 was asserted only against values a JVM test hands to sessionOutcome directly. CI had already run those legs green on the merged commit, so the outstanding item was a gap in coverage rather than a gap in execution. The new case forces a failure with an input that does not exist -- rejected the same way by every FFmpeg build, unlike malformed media -- and asserts the message names the operation, carries the return code, and has detail after it. That last clause is the device-only half. getReturnCode, getFailStackTrace and getAllLogsAsString are native reads; if the log tail came back empty on a device the user would see "Joining failed (1): " with nothing after the colon, and every JVM test would still pass. It deliberately does not pin which detail source wins. On an ordinary non-zero return code FFmpegKit reports no fail stack trace, so stack-trace-first and log-tail-first produce identical text and no assertion here could tell them apart. SessionOutcomeTest pins that, where both sources can be non-blank at once. Claiming it here would be a KDoc asserting more than the test checks. Measured on the local API 33 emulator: 61/61 green with the test, and with both detail sources nulled in ConcatEngine it fails on the intended assertion -- "the message stopped at the return code and told the user nothing, was: 'Joining failed (1): '" -- so the prefix and return code survive the mutation and only the device-only claim goes red. Co-Authored-By: Claude Opus 5 (1M context) --- .../join/ConcatEngineTest.kt | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/app/src/androidTest/java/org/libremediaconverter/join/ConcatEngineTest.kt b/app/src/androidTest/java/org/libremediaconverter/join/ConcatEngineTest.kt index 4a7d6cb..4e9e1a5 100644 --- a/app/src/androidTest/java/org/libremediaconverter/join/ConcatEngineTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/join/ConcatEngineTest.kt @@ -15,6 +15,7 @@ import org.junit.runner.RunWith import org.libremediaconverter.convert.MediaProbe import org.libremediaconverter.convert.StagingNames import org.libremediaconverter.ffmpeg.ConcatEngine +import org.libremediaconverter.ffmpeg.FFmpegEngine import org.libremediaconverter.model.ConcatStrategy import java.io.File @@ -149,6 +150,55 @@ class ConcatEngineTest { ) } + /** + * A failed join tells the user the return code and what FFmpeg said. + * + * **This is the device half of #203/#217**, whose PR closed by noting the join legs had not + * been run. Running them would not have answered it: nothing on either source set drove a real + * join *failure*, so the unified message was asserted only against values a JVM test hands to + * `sessionOutcome` directly. + * + * What is device-only here is that the three reads behind that message work against a real + * native session at all — `getReturnCode`, `getFailStackTrace` and `getAllLogsAsString`. If + * the log tail came back null or empty on a device, the user would get `Joining failed (1): ` + * with nothing after the colon and every JVM test would still pass. + * + * **What this deliberately does not pin is the preference between the two detail sources.** On + * an ordinary non-zero return code FFmpegKit reports no fail stack trace, so the stack-trace- + * first rule and the log-tail-first rule produce the same text and no assertion here can tell + * them apart. That ordering is [SessionOutcomeTest][org.libremediaconverter.ffmpeg.SessionOutcomeTest]'s + * job, where both sources can be non-blank at once. Asserting it here would be a test whose + * KDoc claims more than it checks — the `probeForConcat` mistake wave 3 caught. + * + * The failure is forced with an input that does not exist, which the concat demuxer rejects + * the same way on every FFmpeg build, rather than with malformed media whose handling varies. + */ + @Test + fun aFailedJoinReportsTheReturnCodeAndWhatFFmpegSaid(): Unit = runBlocking { + val missing = File(context.cacheDir, "no_such_clip.mp4").also { it.delete() } + val out = output("joined_failure.mp4") + + val failure = runCatching { + engine.join(listOf(Uri.fromFile(clipA), Uri.fromFile(missing)), out) + }.exceptionOrNull() + + assertTrue( + "a join over a missing input must fail, got $failure", + failure is FFmpegEngine.FFmpegException, + ) + val message = failure?.message.orEmpty() + assertTrue( + "the message must name the operation and carry the return code, was: '$message'", + message.startsWith("Joining failed ("), + ) + // The half a JVM test cannot reach: a real session actually produced detail to show. + val detail = message.substringAfter("): ", "") + assertTrue( + "the message stopped at the return code and told the user nothing, was: '$message'", + detail.isNotBlank(), + ) + } + @Test fun theListFileIsCleanedUpAfterJoining(): Unit = runBlocking { val out = output("joined_cleanup.mp4") From 4d090d9a81e6d988adf21e32de2f2a44c54995a9 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Sat, 5 Sep 2026 21:25:44 -0500 Subject: [PATCH 2/2] Move CLAUDE.md's instrumented counts with the test that changes them The suite goes 60 -> 61 and the gating leg 57 -> 58, because the new join-failure test carries no @FailsOnEmulatorApi37 and so runs on every level including 37. FAILS_ON_EMULATOR_API37_BASELINE stays at 3, checked rather than assumed: e2e-report-shape.sh counts lines matching '^[[:space:]]*@FailsOnEmulatorApi37', which is three annotations -- the fourth occurrence in the tree is the KDoc at Media3EngineTest.kt:195 saying a test is deliberately NOT marked, and the anchor excludes it. Nothing here adds a marker. Carried by this PR rather than the docs one so the file never states a count main does not have. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ad54617..fe48c69 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,11 +76,11 @@ days. Read it as the current answer, and see the git history if you need the old `angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and `swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer table. -- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 60 instrumented +- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 61 instrumented tests cannot pass on that image, for two unrelated reasons: two Media3 hardware transcodes fail inside the emulator's own `c2.goldfish.h264.decoder`, and one SAF test takes the framework down when it rotates the display. All three carry `@FailsOnEmulatorApi37` and run in a separate - `continue-on-error` job; the gating leg runs the other 57. + `continue-on-error` job; the gating leg runs the other 58. That job is still called `E2E API 37 Media3 hardware transcode (advisory)`, which no longer describes everything in it. The name is kept deliberately — it is not a required context and