Compare commits

..
Author SHA1 Message Date
JMR-devandClaude Opus 5 cae050fcfb Bound the wedge diagnostics, so a wedged leg reports as a wedge (#122)
A wedge already reports itself correctly, and then the leg spends half an hour
not saying so. From #151, run 33071570911:

    12:31:29  > Task :app:connectedDebugAndroidTest
    12:48:44  wedged: yes -- gradle was killed after 1200s and never returned
    12:49:20  ::warning::E2E api37 WEDGED (api37)
              ... 36 minutes of nothing ...
    13:25:05  ##[error]The operation was canceled.        <- timeout-minutes: 60
    13:25:09  Terminate orphan process: qemu-system-x86_64-headless, adb, java, java

WEDGE_TIMEOUT=1200 fired exactly as designed. What overran was everything after
it: `capture_wedge` and `dump_diagnostics` probe the device with adb, and the
device they are probing has just finished proving it stopped answering.

WHY `|| true` DID NOT COVER THIS. Every probe carries one, and the file header
says why:

    # Every probe is guarded with `|| true`. A diagnostic must never be the thing
    # that turns a run red.

`|| true` guards a probe that exits non-zero. It does nothing about one that
never exits at all. The discipline was enforced for exit codes and not for time,
and this is the other half of it.

WHAT IT COSTS BEYOND THE 36 MINUTES. The job ends `cancelled` rather than failing
with the wedge's own status, so `gh pr checks` renders a failure with no cause and
the carefully-built `wedged:` row sits above half an hour of silence -- the one
mechanism built to explain a wedge is the least likely to be read. `kill
"$LOGCAT_PID"` never runs either, which is the orphaned adb and qemu above. And a
cancelled required check blocks merges: this one stopped a seven-PR stack.

THE FIX. An `adbq` wrapper -- `timeout -k 5s 20 adb "$@"` -- applied to all 18
probes in `capture_wedge`, `dump_diagnostics`, and the pre-run memory snapshot.
20s is far more than any of them needs on a healthy device and far less than any
costs on a dead one; `-k` because adb itself can ignore the first signal when its
server is wedged.

WHAT IS DELIBERATELY LEFT UNBOUNDED, and it is not everything else by accident:

  - The API 37 SystemUI disable machinery (`adb shell stop`/`start`, the
    `service check` loop). Functional, not diagnostic, and it already carries its
    own verify-and-retry -- see the comment block above it.
  - The backgrounded `adb logcat -v time` stream. It is meant to run for the whole
    leg; bounding it would truncate the log at 20 seconds.

Verified rather than assumed. Wrapper semantics, smoke-tested against a fake adb:
passthrough works, a non-zero exit is preserved (rc=7), a hang is killed at the
deadline (rc=124 after 2s), and `|| true` still swallows that -- so a bounded
probe cannot turn a run red either, which is the property the header demands.

shellcheck clean at the pinned digest over `git ls-files '*.sh'`, actionlint clean
at its pinned digest, `bash -n` clean.

One thing this does NOT do: it does not stop the wedge. #122 is still open for
that. It stops a wedge from being reported as a cancellation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 20:45:38 -05:00
30 changed files with 89 additions and 2771 deletions
+37 -18
View File
@@ -79,6 +79,21 @@ WEDGE_TIMEOUT=1200
# package against `pm list packages -d`, and require a 45 s window with zero new aborts.
# Three rounds, because one is not reliable and the failure is silent.
# ---------------------------------------------------------------------------
# Every device probe below is aimed at an emulator that has already failed, and on the wedge path
# at one that has just finished proving it stopped answering. So each is bounded in time as well as
# in exit status.
#
# `|| true` guards a probe that exits non-zero. It does nothing about one that never exits -- which
# is how #122's wedge path spent 36 minutes after printing its own diagnosis, lost the job to the
# 60-minute cap, and so reported `cancelled` instead of the wedge's own status. The header's rule
# that "a diagnostic must never be the thing that turns a run red" was enforced for exit codes and
# not for time; this is the other half of it.
#
# 20s is far more than any of these needs on a healthy device and far less than any of them costs
# on a dead one. `-k` because adb itself can ignore the first signal when its server is wedged.
ADB_PROBE_TIMEOUT=20
adbq() { timeout -k 5s "$ADB_PROBE_TIMEOUT" adb "$@"; }
count_aborts() { adb logcat -d -b crash 2> /dev/null | grep -c 'hasReadColorBufferDma'; }
systemui_disabled() { adb shell pm list packages -d 2> /dev/null | grep -q 'com.android.systemui'; }
@@ -155,11 +170,11 @@ LOGCAT_PID=$!
dump_diagnostics() {
{
echo "===== E2E api${LABEL} failure diagnostics -- $(date -u +%FT%TZ) ====="
echo "--- adb devices ---"; adb devices -l 2>&1 || true
echo "--- guest memory ---"; adb shell cat /proc/meminfo 2>&1 | grep -E 'MemTotal|MemAvailable|SwapTotal' || true
echo "--- guest storage ---"; adb shell df /data 2>&1 || true
echo "--- is the app even installed? ---"; adb shell pm list packages 2>&1 | grep -a libremedia || true
echo "--- native crashes ---"; adb logcat -d -b crash 2>&1 | tail -80 || true
echo "--- adb devices ---"; adbq devices -l 2>&1 || true
echo "--- guest memory ---"; adbq shell cat /proc/meminfo 2>&1 | grep -E 'MemTotal|MemAvailable|SwapTotal' || true
echo "--- guest storage ---"; adbq shell df /data 2>&1 || true
echo "--- is the app even installed? ---"; adbq shell pm list packages 2>&1 | grep -a libremedia || true
echo "--- native crashes ---"; adbq logcat -d -b crash 2>&1 | tail -80 || true
echo "--- runner: kvm ---"; ls -l /dev/kvm 2>&1 || true
echo "--- runner: memory ---"; free -h 2>&1 || true
echo "--- runner: disk ---"; df -h 2>&1 || true
@@ -167,9 +182,9 @@ dump_diagnostics() {
# Also to the step log, so the common case needs no artifact download.
echo "----- FAILURE SUMMARY (api${LABEL}) -----"
adb shell cat /proc/meminfo 2>&1 | grep -E 'MemTotal|MemAvailable' || true
adbq shell cat /proc/meminfo 2>&1 | grep -E 'MemTotal|MemAvailable' || true
echo "--- native crashes (tail 60) ---"
adb logcat -d -b crash 2>&1 | tail -60 || true
adbq logcat -d -b crash 2>&1 | tail -60 || true
}
capture_wedge() {
@@ -182,35 +197,39 @@ capture_wedge() {
echo "--- running/last instrumented test (logcat TestRunner) ---"
grep -a TestRunner "$LOGCAT_LOG" 2>/dev/null | tail -25 || true
echo "--- boot state ---"
adb shell getprop sys.boot_completed 2>&1 || true
adbq shell getprop sys.boot_completed 2>&1 || true
echo "--- are the binder services published? ---"
for svc in input window activity media.player; do
echo " service check $svc:"; adb shell service check "$svc" 2>&1 || true
echo " service check $svc:"; adbq shell service check "$svc" 2>&1 || true
done
APP_PID="$(adb shell pidof "$APP_ID" 2>/dev/null | tr -d '\r')" || true
TEST_PID="$(adb shell pidof "$TEST_ID" 2>/dev/null | tr -d '\r')" || true
APP_PID="$(adbq shell pidof "$APP_ID" 2>/dev/null | tr -d '\r')" || true
TEST_PID="$(adbq shell pidof "$TEST_ID" 2>/dev/null | tr -d '\r')" || true
echo "--- pids --- app: ${APP_PID:-<none>} test: ${TEST_PID:-<none>}"
# SIGQUIT makes ART dump every thread's stack to logcat and /data/anr. This is what
# distinguishes a deadlocked test from a stuck native encode from a dead device.
echo "--- SIGQUIT thread dumps ---"
for pid in $APP_PID $TEST_PID; do
[ -n "$pid" ] && adb shell kill -3 "$pid" 2>&1 || true
[ -n "$pid" ] && adbq shell kill -3 "$pid" 2>&1 || true
done
sleep 5
echo "--- /data/anr/* ---"
adb shell 'cat /data/anr/* 2>/dev/null' 2>&1 || true
echo "--- dumpsys activity ---"; adb shell dumpsys activity 2>&1 || true
echo "--- dumpsys window ---"; adb shell dumpsys window 2>&1 || true
adbq shell 'cat /data/anr/* 2>/dev/null' 2>&1 || true
echo "--- dumpsys activity ---"; adbq shell dumpsys activity 2>&1 || true
echo "--- dumpsys window ---"; adbq shell dumpsys window 2>&1 || true
# FFmpeg and Media3 both run through MediaCodec; a wedged transcode shows up here.
echo "--- dumpsys media.player ---"; adb shell dumpsys media.player 2>&1 || true
echo "--- dumpsys media.player ---"; adbq shell dumpsys media.player 2>&1 || true
echo "--- logcat -d (tail 400, includes the SIGQUIT dump) ---"
adb logcat -d 2>&1 | tail -400 || true
adbq logcat -d 2>&1 | tail -400 || true
} >> "$WEDGE_LOG" 2>&1 || true
echo "::warning::E2E api${LABEL} WEDGED ($1) -- see the wedge-diagnostics-api${LABEL} artifact"
}
echo "::group::E2E api${LABEL}"
adb shell cat /proc/meminfo 2>&1 | grep -E 'MemTotal|MemAvailable|SwapTotal' || true
# Bounded like the probes in the two diagnostic functions, and for the same reason. This one runs
# against a freshly booted emulator rather than a wedged one, so it is the least likely of them to
# hang -- but it is still a `|| true` diagnostic, and the rule this file now states is that a
# diagnostic must never be the thing that ends the leg.
adbq shell cat /proc/meminfo 2>&1 | grep -E 'MemTotal|MemAvailable|SwapTotal' || true
status=0
# -k 30s SIGKILLs a gradle client that ignores SIGTERM. The wrapper covers ONLY the foreground
+3 -110
View File
@@ -130,9 +130,9 @@ install for code that can never run — and on API 37 the full APK does not fit
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
decision layer, where one branch is one documented user-visible outcome and the metric counts
answers rather than complexity. Every other rule still applies there.
- **Coverage is reported, not gated** — **92.8% of lines (2183/2352), 81.3% of branches
(1091/1342)**, measured 2026-09-02 with `./gradlew :app:jacocoTestReport`, against 584 JVM tests
in 87 classes.
- **Coverage is reported, not gated** — **88.9% of lines (2087/2348), 75.4% of branches
(1011/1340)**, measured 2026-08-29 with `./gradlew :app:jacocoTestReport`, against 546 JVM tests
in 76 classes.
**Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
@@ -174,115 +174,8 @@ install for code that can never run — and on API 37 the full APK does not fit
denominator shrank is not the same claim as one that rises because more branches are tested, and
this entry has a history of explaining its own numbers wrongly.
Then wave 3 (#167-#178) on 2026-09-02 — 88.9% -> 92.8% line, 75.4% -> **81.3%** branch, 546 ->
584 tests, in ten PRs from #179 to #188.
**Its shape is different from the two before it, and the difference is the thing to carry
forward.** Waves 1 and 2 were finding uncovered code. By wave 3 there was not much of that left,
so the gaps were sorted into two kinds before any test was written:
- **coverage gaps** — the line never executes. Filtered to sites where JaCoCo reports `mi > 0`, a
concrete instruction no test runs, which is what separates a real gap from a partial branch on
a compound condition. That filter cut the candidate list roughly in half and was right to.
**Wave 4 found it wrong in both directions, though — use the two filters below instead.**
- **assertion gaps** — JaCoCo is green and nothing checks the answer. `MainActivity`'s rail and
bottom bar were both *executed* by `AppRootRestorationTest` and **transposing them passed the
entire suite**; so did swapping the two progress-notification strings, and swapping `Content`'s
two destinations. No coverage number would ever have found any of the three.
**Wave 4 (2026-09-02) corrected that first filter, and the correction is the reusable part.**
`mi > 0` fails in both directions. It *over-reports* on Compose: `JoinScreen.kt:222` reads
`mi=10` and also `ci=38`, and `JoinStateAffordancesTest` already clicks that Save button and
asserts `save:joined.mp4` — the missed instructions are the synthesized `$changed`/`$dirty`
recomposition-skip path, the same codegen this file already warns about for *branch* counts,
showing up in the instruction count too. And it *under-reports* on warm methods with cold arms:
`ConversionViewModel.cancel()` misses no line, yet `activeWorkId?.let(...)` had only ever been
entered on the null side in 584 tests. Use two filters together instead:
- **`ci == 0`** — the line never executed. This is JaCoCo's own missed-line definition, so it
totals exactly the reported missed-line count and needs no judgement.
- **`ci > 0 && mb > 0` at method level** — a covered method with an arm nothing takes. This is
the only one that finds the `cancel()` shape.
Of wave 4's 251 missed branches, just **18** sat on lines that do execute, so the branch gap and
the line gap are largely the same gap; the second filter is about which of them are reachable.
So **every ticket named the mutation that had to go red, and that was its acceptance criterion
rather than a coverage delta**. It caught **two vacuous tests written in the same session**,
before either shipped:
- a `firstContainerHolding` test asserting a refusal still offered *something*. True, and
useless: the source container is a candidate in its own right, so the list stays non-empty
whatever the fallback does. What it actually buys is the codec the user asked for.
- a staged-delete test scanning for a `"join-"` prefix `StagingNames.forJob` does not produce —
it names files `<jobId>.<ext>`, so the assertion was true of everything.
It also corrected a *third* test that was not vacuous: `probeForConcat`'s KDoc claimed to drive
the `catch` arm, and rethrowing from that catch left it green. That is how the arm turned out to
be unreachable — see the next paragraph. A passing test with a wrong explanation is its own
failure mode.
**A green mutation is only evidence when the mutation is a real change**, which is the mirror
trap: one `classify` mutation stayed green because reordering two arms was semantically
equivalent for every reachable input. A bad mutation and a weak test look identical in the output.
Three things came back **not as the ticket described them**, which is a result rather than a
shortfall:
- `ContainerCapabilities:282`'s `exclude` filter **cannot drop anything**. `repair` always
changes a codec on the shared container — a codec it left alone is one `validate` would not
have refused — and the one non-default `exclude` carries `COPY` while every candidate carries
`NONE`. F4-shaped.
- `probeForConcat`'s catch arm is **unreachable on this runtime**. 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 with
`trackCount = 0`. It stays device-only.
- `JobSnapshots:31`'s missed arm was **not** the `!isFile` one the ticket named — that is already
covered by the `reclaimed` fixture. It was `path == null`: a job carrying no output path at
all. Read the report, not the ticket, when the two disagree.
One item was **included against** the F4 rule rather than exempted by it, and the distinction is
worth having written down since both live in the same function: `ContainerCapabilities:94`
(`accepts(container, VideoCodec.NONE, mode)`) is dead in production today — every caller guards
`NONE` first — and was tested anyway, because its audio twin at `:101` has had a test since #136
and the asymmetry was the argument. The `COPY -> error(...)` arms beside it stay exempt, because
a second line of defence that can be provoked is not one.
Denominators moved here too, in both directions and for two different reasons: 1340 -> 1342
branches from `MediaProbe.merge`, 2348 -> 2352 lines from the `ConcatJoiner` interface. Neither
is new untested code.
And **re-measure before quoting**: this entry was once written quoting 81.4%, measured four hours
earlier, and was already three points stale by the time it was ready to merge.
**Wave 4's read (2026-09-02) moved no number at all, and that is its result.** It was a triage
rather than a test push: twelve tickets (**#192-#203**), four deferred candidates (**#204**), and
five findings (**F6-F10** in `docs/coverage-read-findings.md`). What it establishes is the shape
of what is left, which is different again from wave 3's:
- Of 169 never-executed lines, **81 are native or device edges and stay that way** —
`FFmpegEngine` 33, `Media3Engine` 24, `ConcatEngine` 14, `MainActivity.onCreate` 10 — their
zeroes being the `testDebugUnitTest`-only measurement boundary that #84, #85, #86 and #88 each
recorded before. A further **34 are device-bound only until a seam moves them**:
`AndroidDeviceCodecs` 20 (#194) and the 14 of `MediaProbe`'s 26 that are `readMediaInformation`
(#195). Do not read that second group as exempt — the two tickets exist because it is not.
- Most of the rest is **already closed with a reason on record**, or compiler-generated: default-arg
bridges, DI factory lambdas, synthetic `NoWhenBranchMatchedException` arms, coroutine completion.
- Six of the ten findings in that document are now "no action" or "not a test gap". By this point
the report's remaining red is mostly arms nothing can reach, members nothing calls, and arms a
test *can* reach but cannot pin — and a coverage number tells none of them apart.
**The biggest single gap it found was not a missed line.** `ConversionViewModel.cancel()` and
`JoinViewModel.cancel()` report every line covered; only the null arm of
`activeWorkId?.let(workManager::cancelWorkById)` had ever been entered, so nothing in 584 tests
connected the Cancel button to WorkManager (#192). That is what the second filter above is for.
It also re-opened a mechanism, not a close: #86 and #133 ruled `AndroidDeviceCodecs.probe()` out
**through `ShadowMediaCodecList`**, on the grounds that the builder cannot set `isAlias` or
`canonicalName`. A pure seam does not have that constraint, and #133 did not evaluate one. Read
#194 before re-arguing either way — and note the reason it is worth cutting is not coverage but
that the `runCatching` fallback logs "assuming permissive" while returning empty sets, which makes
`canEncode` and `canDecode` answer *no* for everything.
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
a change that is both needs both.
@@ -24,11 +24,9 @@ import androidx.compose.runtime.saveable.Saver
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.testTag
import androidx.media3.common.util.UnstableApi
import org.libremediaconverter.convert.ConverterScreen
import org.libremediaconverter.join.JoinScreen
import org.libremediaconverter.ui.TestTags
import org.libremediaconverter.ui.theme.LibreMediaConverterTheme
/**
@@ -116,7 +114,7 @@ internal fun AppRoot(
if (useRail) {
Row(modifier = Modifier.fillMaxSize()) {
NavigationRail(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_RAIL)) {
NavigationRail {
Destination.entries.forEach { item ->
NavigationRailItem(
selected = destination == item,
@@ -134,7 +132,7 @@ internal fun AppRoot(
Scaffold(
modifier = Modifier.fillMaxSize(),
bottomBar = {
NavigationBar(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_BAR)) {
NavigationBar {
Destination.entries.forEach { item ->
NavigationBarItem(
selected = destination == item,
@@ -21,11 +21,6 @@ import org.libremediaconverter.model.VideoCodec
* words, "cannot be tested for correctness". It is a hint, not a guarantee, which is
* why the router treats a failed hardware export as a signal to fall back rather
* than trusting this up front.
* - **An enumeration that fails answers no to everything**, which sends every job to
* FFmpeg. Empty sets are not a permissive default: `canEncode` looks a MIME type up in
* [hardwareEncodeMimes] and finds nothing there. That is the intended answer — FFmpeg
* can do whatever Media3 can, only slower — but it is the opposite of what this class
* said until #194, so it is written down rather than left to be re-derived.
*/
class AndroidDeviceCodecs private constructor(
private val hardwareEncodeMimes: Set<String>,
@@ -51,62 +46,22 @@ class AndroidDeviceCodecs private constructor(
fun get(): AndroidDeviceCodecs = cached ?: synchronized(this) { cached ?: probe().also { cached = it } }
/**
* One entry of the platform's codec list, reduced to what the rules below read.
*
* The five booleans and the type list are the whole of what [capabilitiesFrom] needs, and
* none of them can be set on a `MediaCodecInfo` from a test: Robolectric ships
* `MediaCodecInfoBuilder`, but it has no `setIsAlias` and no `setCanonicalName`, which is
* exactly the objection #133 raised against reaching this code through
* `ShadowMediaCodecList`. That objection is about the shadow. It does not apply to a
* function that takes its own entry type, which is why this exists.
*/
internal data class CodecEntry(
val canonicalName: String,
val isAlias: Boolean,
val isEncoder: Boolean,
val isHardwareAccelerated: Boolean,
val isSoftwareOnly: Boolean,
val supportedTypes: List<String>,
)
/**
* The enumeration rules, over entries a caller chooses.
*
* [probe] is the only production caller and supplies the real codec list; a test supplies
* its own, which is the point — the two rules this class's KDoc calls out as easy to get
* wrong, the alias skip and the canonical-name dedup, are unreachable any other way.
*
* **`enumerate` returns a `Sequence`, deliberately.** The `runCatching` has to wrap the
* *iteration* rather than a list built before it, because a `MediaCodecInfo` whose
* properties throw does so partway through — and when that happens the codecs already read
* are kept. Taking a `List` here would move that throw outside the loop and silently turn a
* partial answer into an empty one. That behaviour predates this seam; a `List` parameter
* would have changed it as a side effect of a refactor.
*
* **An enumeration that fails answers restrictively, and that is deliberate.** The sets
* come back empty, and `"video/avc" in emptySet()` is `false`, so [canEncode] and
* [canDecode] both answer no and every job routes to FFmpeg. FFmpeg can do everything
* Media3 can, only slower, so refusing the hardware path is the safe reading of "we could
* not find out what this device supports". This used to log "assuming permissive", which
* described the opposite of what the code does.
*/
internal fun capabilitiesFrom(enumerate: () -> Sequence<CodecEntry>): AndroidDeviceCodecs {
private fun probe(): AndroidDeviceCodecs {
val encoders = mutableSetOf<String>()
val decoders = mutableSetOf<String>()
val seen = mutableSetOf<String>()
runCatching {
enumerate().forEach { entry ->
MediaCodecList(MediaCodecList.REGULAR_CODECS).codecInfos.forEach { info ->
// Aliases point at the same underlying codec; counting both would
// double-count capabilities.
if (entry.isAlias) return@forEach
if (!seen.add(entry.canonicalName)) return@forEach
if (info.isAlias) return@forEach
if (!seen.add(info.canonicalName)) return@forEach
entry.supportedTypes.forEach { mime ->
info.supportedTypes.forEach { mime ->
if (!mime.startsWith("video/")) return@forEach
if (entry.isEncoder) {
if (entry.isHardwareAccelerated && !entry.isSoftwareOnly) {
if (info.isEncoder) {
if (info.isHardwareAccelerated && !info.isSoftwareOnly) {
encoders += mime
}
} else {
@@ -114,32 +69,12 @@ class AndroidDeviceCodecs private constructor(
}
}
}
}.onFailure { Log.w(TAG, "Codec enumeration failed; routing everything to FFmpeg.", it) }
}.onFailure { Log.w(TAG, "Codec enumeration failed; assuming permissive.", it) }
Log.i(TAG, "Hardware video encoders: $encoders")
return AndroidDeviceCodecs(encoders, decoders)
}
/**
* The thin edge: the real codec list, mapped onto [CodecEntry] one at a time.
*
* Lazily, so a property that throws does it inside [capabilitiesFrom]'s `runCatching` and
* on the entry that caused it — see that function's note on why the parameter is a
* `Sequence`.
*/
private fun probe(): AndroidDeviceCodecs = capabilitiesFrom {
MediaCodecList(MediaCodecList.REGULAR_CODECS).codecInfos.asSequence().map { info ->
CodecEntry(
canonicalName = info.canonicalName,
isAlias = info.isAlias,
isEncoder = info.isEncoder,
isHardwareAccelerated = info.isHardwareAccelerated,
isSoftwareOnly = info.isSoftwareOnly,
supportedTypes = info.supportedTypes.toList(),
)
}
}
/**
* `internal` rather than `private` so the cross-check test can ask what a [VideoCodec]
* means here and compare it with what [NAME_TO_MIME] says the same codec's names mean.
@@ -50,42 +50,19 @@ object MediaProbe {
)
fun probe(context: Context, uri: Uri): InputProbe {
val merged = merge(probeWithExtractor(context, uri), probeWithFFprobe(context, uri))
if (merged.kind == InputKind.UNPARSEABLE) {
// Not a failure: an unparseable input is a strong signal that this job belongs on
// FFmpeg. Reporting an unknown codec makes the router say so.
Log.i(TAG, "Neither MediaExtractor nor FFprobe could read $uri; routing to FFmpeg.")
}
return merged
}
val extracted = probeWithExtractor(context, uri)
val info = probeWithFFprobe(context, uri)
/**
* What the two probes together say about one input.
*
* A pure function, and `internal` for the same reason [extractedFrom] is: the precedence rules
* below are the answer to "which probe wins", and until this was pulled out of [probe] the only
* way to ask was to have a real `MediaExtractor` and a real FFprobe **disagree**, which nothing
* on any source set can arrange. `RemuxTest` drives this on a device against committed
* fixtures, but only ever with one probe answering and the other agreeing or also failing --
* so every elvis here was taken in one direction and never the other.
*
* The rules, each of which is a decision rather than an accident:
*
* - **The extractor wins on codecs.** It is the platform's own view of what it can decode,
* which is the thing the router is about to ask about. FFprobe's name for the same track can
* differ, and the copy planner keys off these strings.
* - **FFprobe alone reports the container.** `MediaExtractor` cannot, which is why [InputProbe]
* carries a nullable one and `CopyPlanner` treats null as "container unknown".
* - **Duration is the larger of the two**, not the first non-zero. Either probe can report zero
* for a file the other times correctly, and a zero duration makes the FFmpeg progress
* percentage undefined.
*/
internal fun merge(extracted: Extracted?, info: FFprobeInfo?): InputProbe {
val videoCodec = extracted?.videoCodec ?: info?.videoCodec
val audioCodec = extracted?.audioCodec ?: info?.audioCodec
val kind = classify(extracted, info)
if (kind == InputKind.UNPARSEABLE) return UNREADABLE
if (kind == InputKind.UNPARSEABLE) {
// Not a failure: an unparseable input is a strong signal that this job belongs on
// FFmpeg. Reporting an unknown codec makes the router say so.
Log.i(TAG, "Neither MediaExtractor nor FFprobe could read $uri; routing to FFmpeg.")
return UNREADABLE
}
return InputProbe(
videoCodec = videoCodec,
@@ -106,7 +83,7 @@ object MediaProbe {
* audio file and a corrupt file indistinguishable. The source-info card cannot describe either
* honestly until they are separate, and neither can the copy planner.
*/
internal fun classify(extracted: Extracted?, info: FFprobeInfo?): InputKind = when {
private fun classify(extracted: Extracted?, info: FFprobeInfo?): InputKind = when {
info?.isImage == true -> InputKind.IMAGE
extracted == null && info == null -> InputKind.UNPARSEABLE
(extracted?.videoCodec ?: info?.videoCodec) != null -> InputKind.VIDEO
@@ -185,11 +162,7 @@ object MediaProbe {
}
}
/**
* `internal` rather than `private` for the same reason [Extracted] is, and it should have been
* from the start: [merge] cannot be named from a test while half its signature is private.
*/
internal class FFprobeInfo(
private class FFprobeInfo(
val container: Container?,
val videoCodec: String?,
val audioCodec: String?,
@@ -221,39 +194,12 @@ object MediaProbe {
null
}
/**
* The thin edge: spawn FFprobe, hand what it said to [ffprobeInfoFrom].
*
* Everything device-bound is on this line and the null check under it. What FFprobe *said* is a
* `MediaInformation`, which is an ordinary object over a `JSONObject` — so the reading of it is
* a decision a test can choose the inputs for, and it lives below rather than here.
*/
private fun readMediaInformation(path: String): FFprobeInfo? =
FFprobeKit.getMediaInformation(path).getMediaInformation()?.let(::ffprobeInfoFrom)
private fun readMediaInformation(path: String): FFprobeInfo? {
// ffmpeg-kit-next is compiled from Kotlin with private backing fields, so these have to go
// through the Java getters rather than property syntax.
val info: MediaInformation = FFprobeKit.getMediaInformation(path).getMediaInformation()
?: return null
/**
* What FFprobe's answer means, as a function of the answer alone.
*
* `internal` for the same reason [Extracted] and [FFprobeInfo] are: a test cannot name it
* otherwise, and the JVM test source set is a friend of `main`.
*
* **JVM-safe, verified rather than assumed.** `javap` over the committed AAR's runtime jar:
* `MediaInformation(JSONObject, List<StreamInformation>, List<Chapter>)` and
* `StreamInformation(JSONObject)` are plain public constructors, and neither class's `<clinit>`
* touches the native library — so a test builds its own without `libffmpegkit` being present.
* That is the whole reason this split is worth making: `readMediaInformation` was 114 missed
* instructions and 24 missed branches, of which exactly one line needed a device.
*
* The subtle part is the **second argument to [containerFrom]**. `matroska,webm` is reported
* for both MKV and WebM — they share a demuxer — so the video codec is the only thing that
* separates them, and dropping it silently turns every VP9 WebM into an MKV. `containerFrom`
* has thirty-three covered branches of its own and none of them can notice that, because the
* mistake is at the call rather than in the callee.
*
* ffmpeg-kit-next is compiled from Kotlin with private backing fields, so these go through the
* Java getters rather than property syntax.
*/
internal fun ffprobeInfoFrom(info: MediaInformation): FFprobeInfo {
val streams = info.getStreams().orEmpty()
val video = streams.firstOrNull { it.getType() == "video" }
val audio = streams.firstOrNull { it.getType() == "audio" }
@@ -4,7 +4,6 @@ import android.content.Context
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import org.libremediaconverter.codec.AndroidDeviceCodecs
import org.libremediaconverter.ffmpeg.ConcatEngine
import org.libremediaconverter.ffmpeg.FFmpegEngine
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.DeviceCodecs
@@ -41,27 +40,6 @@ interface SoftwareTranscoder {
)
}
/**
* The join path. Implemented by [org.libremediaconverter.ffmpeg.ConcatEngine].
*
* Added last of the three, and the gap it closes was measured rather than guessed:
* `PerJobStagingTest`'s KDoc records that reverting `ConcatWorker` to a constant staging name left
* all 257 tests green, because nothing in the JVM suite can get past a `ConcatEngine` constructed
* in place. Everything after that line -- the failure mapping, the message fallback, the staged
* delete -- was untested on every source set.
*
* The result type stays nested in the implementation rather than being lifted here. Moving it would
* touch every call site to buy nothing: what a test needs is the ability to *not* run FFmpeg, and
* that is the method, not the type.
*/
interface ConcatJoiner {
suspend fun join(
inputs: List<Uri>,
output: File,
format: OutputFormat = OutputFormat.MP4_H264,
): ConcatEngine.Result
}
/**
* The seam that lets tests force failure paths.
*
@@ -91,9 +69,6 @@ object ConversionDependencies {
@Volatile
var software: () -> SoftwareTranscoder = { FFmpegEngine() }
@Volatile
var concat: (Context) -> ConcatJoiner = { ConcatEngine(it) }
@Volatile
var publisher: (Context) -> OutputPublisher = { OutputPublisher(it) }
@@ -128,7 +103,6 @@ object ConversionDependencies {
fun reset() {
hardware = { Media3Engine(it) }
software = { FFmpegEngine() }
concat = { ConcatEngine(it) }
publisher = { OutputPublisher(it) }
deviceCodecs = { AndroidDeviceCodecs.get() }
probe = { context, uri -> MediaProbe.probe(context, uri) }
@@ -7,7 +7,6 @@ import com.arthenica.ffmpegkit.FFmpegKit
import com.arthenica.ffmpegkit.FFmpegKitConfig
import com.arthenica.ffmpegkit.ReturnCode
import kotlinx.coroutines.suspendCancellableCoroutine
import org.libremediaconverter.convert.ConcatJoiner
import org.libremediaconverter.convert.MediaProbe
import org.libremediaconverter.convert.StagingNames
import org.libremediaconverter.model.ConcatPlanner
@@ -25,11 +24,11 @@ import kotlin.coroutines.resumeWithException
* reliably fail when they differ — it can emit a file whose later segments are
* garbled. See [ConcatPlanner].
*/
class ConcatEngine(private val context: Context) : ConcatJoiner {
class ConcatEngine(private val context: Context) {
data class Result(val strategy: ConcatStrategy, val output: File)
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): Result {
suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat = OutputFormat.MP4_H264): Result {
require(inputs.size >= 2) { "Joining needs at least two files." }
val paths = inputs.map { uri ->
@@ -56,20 +56,6 @@ object TestTags {
*/
const val RETRY_SAVE: String = "action.retrySave"
/**
* The adaptive shell around both screens -- `AppRoot`'s two layouts.
*
* Named because there is no other way to tell them apart from a test. Both render the same two
* destinations with the same labels and the same selection state, so every assertion that could
* be written without these tags is satisfied by either layout, and transposing the two bodies
* passed the whole suite. Exactly one of the two exists at a time, which is what makes
* `assertExists` / `assertDoesNotExist` on this pair a statement about the width class.
*/
object Shell {
const val NAVIGATION_RAIL: String = "shell.navigationRail"
const val NAVIGATION_BAR: String = "shell.navigationBar"
}
/** `ConverterScreen`. */
object Converter {
const val CHOOSE_FILE: String = "converter.chooseFile"
@@ -15,6 +15,7 @@ import kotlinx.coroutines.CancellationException
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.InputQuery
import org.libremediaconverter.convert.StagingNames
import org.libremediaconverter.ffmpeg.ConcatEngine
import org.libremediaconverter.model.OutputFormat
/**
@@ -36,7 +37,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
override suspend fun doWork(): Result {
val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse)
?: return Result.failure(workDataOf(KEY_ERROR to NO_INPUTS_MESSAGE))
?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
if (uris.size < 2) {
return Result.failure(workDataOf(KEY_ERROR to TOO_FEW_INPUTS_MESSAGE))
}
@@ -76,7 +77,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
),
)
val result = ConversionDependencies.concat(applicationContext).join(uris, staged, format)
val result = ConcatEngine(applicationContext).join(uris, staged, format)
Result.success(
workDataOf(
KEY_OUTPUT_PATH to staged.absolutePath,
@@ -147,16 +148,6 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
* Here rather than in the ViewModel because the rule is the worker's: `request(...)` takes
* a `List<Uri>` and checks nothing about its length, so this is the guard that always runs.
*/
/**
* A job carrying no input array at all -- a downgrade, or a queue entry from a build that
* spelled the key differently.
*
* A constant rather than the literal it was, for the convention #158 established: a message
* the user can see is named once, so a test asserts the same string the worker writes
* rather than a copy of it that can drift.
*/
const val NO_INPUTS_MESSAGE: String = "No input files."
const val TOO_FEW_INPUTS_MESSAGE: String = "Pick at least two files to join."
/**
@@ -1,129 +0,0 @@
package org.libremediaconverter
import androidx.activity.ComponentActivity
import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import org.junit.After
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
/**
* Which navigation affordance the shell actually renders, and which screen it actually shows.
*
* Assertion gaps rather than coverage gaps, both of them, and that is why they lasted.
* `AppRootRestorationTest` already drives `AppRoot` at `Compact` and `Expanded`, so JaCoCo is green
* on `useRail` -- but it asserts only that the *selected tab* survives recreation, through a stub
* `content` composable. Nothing anywhere queried for a rail or a bar, and nothing rendered the real
* screens. Two consequences, both measured before this file existed:
*
* - **Transposing the `NavigationRail` and `NavigationBar` bodies passed the entire suite.**
* - **Transposing `Content`'s two arms passed it too** -- a tablet showing the phone chrome, or the
* Convert tab opening the Join screen, and 546 tests with nothing to say about either.
*
* `AppRoot`'s own KDoc is why this matters more than it looks: from targetSdk 37 the app is resized
* and rotated whether or not it is ready, so the width class is not a preference, it is whatever
* the system hands over.
*
* ## Two things this needed that the rest of the suite does not
*
* **`createAndroidComposeRule`, not `createComposeRule`.** Rendering `AppRoot` with its *default*
* content reaches `ConverterScreen`'s `viewModel = viewModel()`, which needs a
* `ViewModelStoreOwner`; the plain rule supplies none. It works because both ViewModels are
* `@JvmOverloads constructor(app: Application, …)`, so `AndroidViewModelFactory` can build them,
* and because `app/build.gradle.kts` already puts `ui-test-manifest`'s `ComponentActivity` in the
* merged manifest the unit tests build against -- which that file says in terms.
*
* **Tags on the two bars.** They are in `TestTags`, applied inside `main`, for the reason that
* file's KDoc gives: a tag the test hands down proves only that the test set it.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class AdaptiveShellTest {
@get:Rule
val composeRule = createAndroidComposeRule<ComponentActivity>()
@Before
fun setUp() {
val app = RuntimeEnvironment.getApplication()
installTestWorkManager(app, Data.EMPTY)
// The real screens are composed here, so their ViewModels are real too. Neither test is
// about probing or publishing; left alone they would reach the FFprobe loader and this
// machine's codec list, and decide things no assertion mentions.
ConversionDependencies.probe = { _, _ -> InputProbe() }
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `a phone gets the bottom bar and a tablet gets the rail`() {
setShell(WindowWidthSizeClass.Compact)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertDoesNotExist()
}
@Test
fun `an expanded window gets the rail`() {
setShell(WindowWidthSizeClass.Expanded)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
}
/**
* The width class no test had ever passed.
*
* `useRail` is `!= Compact`, so Medium takes the rail with Expanded. Narrowing it to
* `== Expanded` is a one-character change that breaks every tablet and unfolded foldable and
* nothing else -- and until this test, nothing in either source set used `Medium` at all.
*/
@Test
fun `a medium window is a rail window, not a phone`() {
setShell(WindowWidthSizeClass.Medium)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
}
/**
* The mapping every other test stubs out: which screen each destination actually opens.
*
* Matched on each screen's own "choose a file" affordance rather than on a title, because those
* tags are applied by the screens themselves -- so this fails if the destinations are
* transposed, and it fails for the right reason.
*/
@Test
fun `Convert opens the converter and Join opens the join screen`() {
setShell(WindowWidthSizeClass.Compact)
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertExists()
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertDoesNotExist()
composeRule.onNodeWithText(Destination.JOIN.label).performClick()
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertExists()
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertDoesNotExist()
}
/** [AppRoot] with its real content, which is the half nothing else composes. */
private fun setShell(width: WindowWidthSizeClass) {
composeRule.setContent { AppRoot(width) }
}
}
@@ -1,201 +0,0 @@
package org.libremediaconverter.codec
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.VideoCodec
import org.robolectric.RobolectricTestRunner
/**
* The rules `AndroidDeviceCodecs.probe()` applies to the platform's codec list.
*
* ## Why this is not a third run of the #86/#133 spike
*
* #86 closed `probe()` as device-bound. #133 re-opened the question with
* `ShadowMediaCodecList` in hand and closed it again, for a reason that was right about what it
* was answering: `MediaCodecInfoBuilder` "has no `setIsAlias` and no `setCanonicalName`, so the
* alias skip and the canonical-name dedup — the two things the class's KDoc calls out as easy to
* get wrong — are not reachable through it."
*
* **That objection is about the shadow.** It does not apply to a function that takes its own entry
* type, which is what `capabilitiesFrom` now does. The half #133 named as unreachable is the half
* this file spends most of its cases on.
*
* ## What made the seam worth cutting, which is not coverage
*
* The `runCatching` fallback logged *"assuming permissive"* and returned empty sets — and empty
* sets are **restrictive**: `"video/avc" in emptySet()` is `false`, so `canEncode` and `canDecode`
* both answer no and every job routes to FFmpeg. The code was right and the message described the
* opposite of it. That is pinned below, so whichever reading a future change takes, it has to say
* so out loud.
*
* Robolectric only because `capabilitiesFrom` logs what it found; the rules themselves are pure.
*/
@RunWith(RobolectricTestRunner::class)
class CodecEnumerationTest {
/**
* The alias skip, in the one arrangement where it is observable — and finding that arrangement
* is the whole of this test.
*
* A first attempt listed the alias *after* the codec it aliases and passed with the skip
* deleted, because `canonicalName` is shared and the dedup below catches the second entry
* either way. The two rules overlap, so a fixture that does not separate them tests neither.
*
* What separates them is **order**. `MediaCodecInfo.getCanonicalName()` on an alias returns the
* underlying codec's name, so an alias arriving first claims that name in `seen` and has its
* own `supportedTypes` credited — and then the real codec is dropped by the dedup. Without the
* alias skip the device is described by whichever entry the platform happened to list first.
*
* That also says what the rule is worth. With a `Set` accumulator, an alias declaring the same
* types as its codec changes nothing whichever order they arrive in; the skip earns its place
* only when the two disagree, which is exactly when believing the wrong one matters.
*/
@Test
fun `an alias listed before the codec it aliases does not describe the device`() {
val codecs = capabilities(
entry("c2.qti.avc.encoder", encoder = true, types = listOf(HEVC), alias = true),
entry("c2.qti.avc.encoder", encoder = true, types = listOf(AVC)),
)
assertTrue("the real codec's types are the device's", codecs.canEncode(VideoCodec.H264))
assertFalse(
"an alias must not be credited with types the codec it aliases never claimed",
codecs.canEncode(VideoCodec.H265),
)
}
@Test
fun `two entries sharing a canonical name are read once`() {
val codecs = capabilities(
entry("c2.qti.avc.encoder", encoder = true, types = listOf(AVC)),
entry("c2.qti.avc.encoder", encoder = true, types = listOf(HEVC)),
)
assertEquals(setOf(AVC), codecs.hardwareEncoders())
}
/**
* Both halves of the hardware predicate, one arm at a time.
*
* A vendor may declare a codec hardware-accelerated *and* software-only; the class KDoc is
* explicit that the first flag "cannot be tested for correctness", so the second is what stops
* a mislabelled software encoder being treated as the fast path.
*/
@Test
fun `an encoder counts as hardware only when it is accelerated and not software-only`() {
assertEquals(
setOf(AVC),
capabilities(entry("hw", encoder = true, accelerated = true, types = listOf(AVC))).hardwareEncoders(),
)
assertEquals(
emptySet<String>(),
capabilities(entry("sw", encoder = true, accelerated = false, types = listOf(AVC))).hardwareEncoders(),
)
assertEquals(
"a codec claiming both must not be trusted as hardware",
emptySet<String>(),
capabilities(
entry("both", encoder = true, accelerated = true, softwareOnly = true, types = listOf(AVC)),
).hardwareEncoders(),
)
}
/**
* Decoders are collected regardless of the hardware flags, and that asymmetry is the design.
*
* `canDecode` asks whether the platform can read the input at all — a software decoder answers
* that as well as a hardware one. `canEncode` asks whether the *fast path* exists, which is a
* different question and why only encoders are filtered.
*/
@Test
fun `a software decoder still counts as something the platform can read`() {
val codecs = capabilities(
entry(
"c2.android.avc.decoder",
encoder = false,
accelerated = false,
softwareOnly = true,
types = listOf(AVC),
),
)
assertTrue(codecs.canDecode("h264"))
}
@Test
fun `audio types are ignored on both sides`() {
val codecs = capabilities(
entry("aac.encoder", encoder = true, accelerated = true, types = listOf("audio/mp4a-latm")),
entry("aac.decoder", encoder = false, types = listOf("audio/mp4a-latm")),
)
assertEquals(emptySet<String>(), codecs.hardwareEncoders())
// Not "the platform cannot decode AAC" -- `canDecode` is asked about *video* codec names,
// and an unknown name is answered permissively. The point is that nothing audio reached
// either set.
assertTrue("an unknown name stays permissive", codecs.canDecode("something-nobody-named"))
}
/**
* The failure fallback, pinned as the restrictive answer it actually is.
*
* #194 decided this rather than assuming it: the code stays, the message changes. If a later
* change wants the permissive reading its old log line described, this test is what makes that
* a decision instead of a drift.
*/
@Test
fun `an enumeration that fails sends every job to FFmpeg`() {
val codecs = AndroidDeviceCodecs.capabilitiesFrom { error("MediaCodecList exploded") }
assertFalse("a failed enumeration must not claim a hardware encoder", codecs.canEncode(VideoCodec.H264))
assertFalse(codecs.canDecode("h264"))
assertEquals(emptySet<String>(), codecs.hardwareEncoders())
}
/**
* A list that throws partway keeps what it already read.
*
* This predates the seam — `runCatching` has always wrapped the iteration rather than a list
* built before it — and it is asserted here because the seam is where it could quietly have
* been lost. Taking a `List` instead of a `Sequence` would move the throw outside the loop and
* turn this partial answer into an empty one, with no test to notice.
*/
@Test
fun `codecs read before a failing entry are kept`() {
val codecs = AndroidDeviceCodecs.capabilitiesFrom {
sequence {
yield(entry("good", encoder = true, accelerated = true, types = listOf(AVC)))
error("the sixth codec's properties threw")
}
}
assertEquals(setOf(AVC), codecs.hardwareEncoders())
}
private fun capabilities(vararg entries: AndroidDeviceCodecs.Companion.CodecEntry) =
AndroidDeviceCodecs.capabilitiesFrom { entries.asSequence() }
private fun entry(
canonicalName: String,
encoder: Boolean,
accelerated: Boolean = true,
softwareOnly: Boolean = false,
alias: Boolean = false,
types: List<String>,
) = AndroidDeviceCodecs.Companion.CodecEntry(
canonicalName = canonicalName,
isAlias = alias,
isEncoder = encoder,
isHardwareAccelerated = accelerated,
isSoftwareOnly = softwareOnly,
supportedTypes = types,
)
private companion object {
const val AVC = "video/avc"
const val HEVC = "video/hevc"
}
}
@@ -7,7 +7,6 @@ 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
/**
@@ -134,38 +133,6 @@ 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"))
@@ -1,180 +0,0 @@
package org.libremediaconverter.convert
import android.app.Application
import androidx.media3.common.util.UnstableApi
import androidx.work.OneTimeWorkRequestBuilder
import androidx.work.WorkInfo
import androidx.work.WorkManager
import androidx.work.workDataOf
import kotlinx.coroutines.Dispatchers
import org.junit.After
import org.junit.Assert.assertEquals
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.InputProbe
import org.libremediaconverter.work.ConcatWorker
import org.libremediaconverter.work.ConversionWorker
import org.libremediaconverter.work.JobTags
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.util.UUID
import java.util.concurrent.TimeUnit
/**
* That `cancel()` cancels the job, on both screens.
*
* ## Why this was missing, which is the interesting part
*
* Both `cancel()` methods are one line — `activeWorkId?.let(workManager::cancelWorkById)` — and
* **JaCoCo reports every line of both as covered**. `SettingsEditsTest`'s
* `cancelling with no active job does nothing rather than throwing` runs the method, and its own
* comment names which half it drives: "`activeWorkId?.let(...)` -- the null side". The other side
* had never been entered, and `JoinViewModel.cancel()` had no test at all.
*
* So no line-level coverage filter could see this. What surfaces it is a method-level read —
* `mi=11, ci=7, mb=1, cb=1` on both — a covered method with an arm nothing takes. That is the
* second of the two filters #194 records, and this is the gap that argued for it.
*
* The affordance tests are not this. `ConverterStateAffordancesTest` and `JoinStateAffordancesTest`
* click `TestTags.CANCEL` and assert the *action* fires into a stub; `ScreenWiringTest` asserts the
* action calls `viewModel.cancel()`. Both halves were pinned and the join between them was not, so
* nothing in 584 tests connected the button to WorkManager.
*
* ## Why the job is enqueued with a delay
*
* The test WorkManager runs on a `SynchronousExecutor`, so an ordinary request finishes inline —
* which is exactly why only the null half was ever covered: by the time a test could call
* `cancel()`, `convert()`'s job was already terminal. `setInitialDelay` is what `TestScheduler`
* honours, so the job sits in `ENQUEUED` until the test lets it go, and it never does.
*
* **Production never sets a delay**, so the request is built here rather than through
* `ConversionWorker.request`. The *state* is not synthetic: `ENQUEUED` at `runAttemptCount == 0` is
* what every job passes through before the scheduler picks it up, `Reattachment.choose` ranks it
* `QUEUED`, and `conversionStateFrom` maps it to `Converting(input, 0)`. The delay changes how long
* the job stays in a real state, not which state it is in.
*
* ## What is asserted, and in which order
*
* WorkManager's own record first, then the screen. The screen alone would be a weaker claim than it
* looks: `CANCELLED` maps to `Idle` for a reattached job, and `Idle` is also where a ViewModel that
* did nothing at all would sit.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class CancelReachesWorkManagerTest {
private lateinit var app: Application
private lateinit var workManager: WorkManager
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
ConversionDependencies.publisher = { RecordingPublisher(app) }
ConversionDependencies.probe = { _, _ -> InputProbe() }
installTestWorkManager(app, workDataOf())
workManager = WorkManager.getInstance(app)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `cancelling a queued conversion cancels that job`() {
val id = enqueueQueuedConversion()
val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
awaitState(viewModel.state, "Converting") { it is ConversionState.Converting }
viewModel.cancel()
assertEquals(
"Cancel must reach WorkManager, not just the screen",
WorkInfo.State.CANCELLED,
stateOf(id),
)
awaitState(viewModel.state, "Idle") { it is ConversionState.Idle }
}
@Test
fun `cancelling a queued join cancels that job`() {
val id = enqueueQueuedJoin()
val viewModel = JoinViewModel(app, Dispatchers.Unconfined)
awaitState(viewModel.state, "Joining") { it is JoinState.Joining }
viewModel.cancel()
assertEquals(
"Cancel must reach WorkManager, not just the screen",
WorkInfo.State.CANCELLED,
stateOf(id),
)
awaitState(viewModel.state, "Idle") { it is JoinState.Idle }
}
/**
* The negative that bounds both: cancelling must cancel the job the screen is showing, and only
* that one.
*
* Without this, `cancel()` could cancel everything in the queue — `cancelAllWork()` in place of
* `cancelWorkById(activeWorkId)` — and both tests above would still pass.
*/
@Test
fun `cancelling one conversion leaves another queued job alone`() {
val bystander = enqueueQueuedConversion(displayName = "beach.mp4")
val id = enqueueQueuedConversion(displayName = "holiday.mp4")
val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
val converting = awaitState(viewModel.state, "Converting") { it is ConversionState.Converting }
val onScreen = (converting as ConversionState.Converting).input.displayName
viewModel.cancel()
// Which of the two the ViewModel reattached to is the query's business, not this test's --
// the comparator leaves queued jobs tied deliberately, per Reattachment's ordering notes.
// So assert the shape rather than the identity: exactly one is cancelled, and the other is
// untouched.
val cancelled = listOf(id, bystander).filter { stateOf(it) == WorkInfo.State.CANCELLED }
assertEquals(
"exactly one job may be cancelled, with $onScreen on screen",
1,
cancelled.size,
)
}
private fun stateOf(id: UUID): WorkInfo.State =
requireNotNull(workManager.getWorkInfoById(id).get()) { "no WorkInfo for $id" }.state
/**
* A conversion sitting in the queue, which is where every job starts.
*
* Built by hand rather than through `ConversionWorker.request` for the reason in the class
* KDoc; the display-name tag is included because `reattach()` reads it for the file card, and a
* job without one would exercise the `UNKNOWN_INPUT_NAME` fallback instead of this test's
* subject.
*/
private fun enqueueQueuedConversion(displayName: String = "holiday.mp4"): UUID {
val request = OneTimeWorkRequestBuilder<ConversionWorker>()
.addTag(JobTags.displayName(displayName))
.setInitialDelay(QUEUE_HOLD_HOURS, TimeUnit.HOURS)
.build()
workManager.enqueue(request).result.get()
return request.id
}
private fun enqueueQueuedJoin(inputCount: Int = 2): UUID {
val request = OneTimeWorkRequestBuilder<ConcatWorker>()
.addTag(JobTags.inputCount(inputCount))
.setInitialDelay(QUEUE_HOLD_HOURS, TimeUnit.HOURS)
.build()
workManager.enqueue(request).result.get()
return request.id
}
private companion object {
/** Long enough that `TestScheduler` never releases the job during a test run. */
const val QUEUE_HOLD_HOURS = 1L
}
}
@@ -1,192 +0,0 @@
package org.libremediaconverter.convert
import com.arthenica.ffmpegkit.MediaInformation
import com.arthenica.ffmpegkit.StreamInformation
import org.json.JSONObject
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.Container
import org.robolectric.RobolectricTestRunner
/**
* What FFprobe's answer means, read as a function of the answer alone.
*
* `readMediaInformation` was 114 missed instructions and 24 missed branches — the second-biggest
* block on the wave-4 report — of which **exactly one line needed a device**:
*
* ```kotlin
* FFprobeKit.getMediaInformation(path).getMediaInformation()
* ```
*
* Everything after it reads an ordinary object. `javap` over the committed AAR's runtime jar:
* `MediaInformation(JSONObject, List<StreamInformation>, List<Chapter>)` and
* `StreamInformation(JSONObject)` are plain public constructors, and neither class's `<clinit>`
* loads the native library — so the fixtures below are built without `libffmpegkit` present.
*
* ## The one that matters
*
* `containerFrom(formatName, video?.getCodec())`. FFprobe reports `matroska,webm` for **both** MKV
* and WebM, because they share a demuxer, so the video codec is the only thing separating them.
* `containerFrom` has thirty-three covered branches of its own and not one of them can notice the
* argument being dropped — the mistake would be at the call, not in the callee, and every existing
* `containerFrom` test would stay green while every VP9 WebM quietly became an MKV.
*
* Robolectric only for `org.json`, which is a stub in a plain JVM test.
*/
@RunWith(RobolectricTestRunner::class)
class FFprobeMappingTest {
@Test
fun `the video codec decides between matroska and webm`() {
assertEquals(
Container.WEBM,
MediaProbe.ffprobeInfoFrom(info("matroska,webm", stream("video", "vp9"))).container,
)
assertEquals(
Container.MKV,
MediaProbe.ffprobeInfoFrom(info("matroska,webm", stream("video", "h264"))).container,
)
}
/**
* The same format name with no video stream at all, which is what makes the case above about
* the *argument* rather than about the format string.
*/
@Test
fun `a matroska container with no video track cannot be told from webm and is not guessed`() {
val read = MediaProbe.ffprobeInfoFrom(info("matroska,webm", stream("audio", "opus")))
assertEquals(Container.MKV, read.container)
assertNull(read.videoCodec)
}
@Test
fun `the first stream of each type wins`() {
val read = MediaProbe.ffprobeInfoFrom(
info(
"mov,mp4,m4a,3gp,3g2,mj2",
stream("video", "h264", width = 1920, height = 1080),
stream("video", "hevc", width = 640, height = 480),
stream("audio", "aac"),
stream("audio", "mp3"),
),
)
assertEquals("h264", read.videoCodec)
assertEquals("aac", read.audioCodec)
assertEquals(1920, read.width)
assertEquals(1080, read.height)
}
/**
* Dimensions come from the stream the codec came from, not from whichever stream has some.
*
* The fixture is deliberately awkward: the chosen video stream carries **no** dimensions and a
* later one does. That is a real shape — FFprobe omits `width`/`height` for a stream it could
* not measure — and it is the only arrangement that separates the two readings.
*
* A first version of this file asserted the dimensions inside the case above, where the chosen
* stream was also the first one carrying any. Replacing `video?.getWidth()` with
* `streams.firstNotNullOfOrNull { it.getWidth() }` gave the same answer there and **the
* mutation survived**. It reddens here.
*/
@Test
fun `a video stream with no dimensions reports none rather than borrowing another stream's`() {
val read = MediaProbe.ffprobeInfoFrom(
info(
"mov,mp4,m4a,3gp,3g2,mj2",
stream("video", "h264"),
stream("video", "hevc", width = 640, height = 480),
),
)
assertEquals("h264", read.videoCodec)
assertEquals(0, read.width)
assertEquals(0, read.height)
}
/**
* Stream order is the file's, not a promise. An audio-first container must read the same as a
* video-first one.
*/
@Test
fun `an audio track listed first does not become the video track`() {
val read = MediaProbe.ffprobeInfoFrom(
info("mov,mp4,m4a,3gp,3g2,mj2", stream("audio", "aac"), stream("video", "h264")),
)
assertEquals("h264", read.videoCodec)
assertEquals("aac", read.audioCodec)
}
@Test
fun `a duration in seconds becomes milliseconds`() {
assertEquals(12_345L, MediaProbe.ffprobeInfoFrom(info("mp4", duration = "12.345")).durationMs)
}
/**
* Both ways a duration can be absent, and neither may throw.
*
* FFprobe reports `"N/A"` for a stream it could not measure, and omits the key entirely for
* some containers. `toDoubleOrNull` is what keeps the second from being an exception on the
* file-pick path, where there is no user-visible failure to report it as.
*/
@Test
fun `a duration that is not a number is no duration rather than a crash`() {
assertEquals(0L, MediaProbe.ffprobeInfoFrom(info("mp4", duration = "N/A")).durationMs)
assertEquals(0L, MediaProbe.ffprobeInfoFrom(info("mp4", duration = null)).durationMs)
}
@Test
fun `a file with no streams reports nothing rather than defaults that look measured`() {
val read = MediaProbe.ffprobeInfoFrom(info("mp4"))
assertNull(read.videoCodec)
assertNull(read.audioCodec)
assertEquals(0, read.width)
assertEquals(0, read.height)
}
@Test
fun `an image format is reported as one`() {
assertTrue(MediaProbe.ffprobeInfoFrom(info("png_pipe", stream("video", "png"))).isImage)
assertFalse(MediaProbe.ffprobeInfoFrom(info("mp4", stream("video", "h264"))).isImage)
}
private fun stream(type: String, codec: String, width: Int? = null, height: Int? = null) = StreamInformation(
JSONObject().apply {
put(StreamInformation.KEY_TYPE, type)
put(StreamInformation.KEY_CODEC, codec)
width?.let { put(StreamInformation.KEY_WIDTH, it) }
height?.let { put(StreamInformation.KEY_HEIGHT, it) }
},
)
/**
* The format properties are **nested** under `"format"`, which is how FFprobe reports them and
* what `MediaInformation` reads: `getFormat()` resolves through `getStringFormatProperty`, not
* off the top-level object. A first version of this helper put the keys at the top level and
* every format-dependent case failed with a null container, which is worth recording here so
* the next fixture does not have to rediscover it.
*
* Streams are the other half and are *not* nested — they come from the constructor argument.
*/
private fun info(formatName: String, vararg streams: StreamInformation, duration: String? = "1.0") =
MediaInformation(
JSONObject().apply {
put(
MediaInformation.KEY_FORMAT_PROPERTIES,
JSONObject().apply {
put(MediaInformation.KEY_FORMAT, formatName)
duration?.let { put(MediaInformation.KEY_DURATION, it) }
},
)
},
streams.toList(),
emptyList(),
)
}
@@ -205,34 +205,6 @@ 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.
@@ -1,99 +0,0 @@
package org.libremediaconverter.convert
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.AudioPlan
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.CopyPlanner
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.model.VideoPlan
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.concurrent.CancellationException
/**
* A job that reached Media3 with a container Media3 cannot mux.
*
* [Media3Muxers]' own KDoc names the defect this guards: *"the router claimed five containers while
* the engine silently wrote MP4 for all of them."* `factoryFor` answers null for fourteen of the
* app's containers, and `buildTransformer` turns that null into a failed job rather than letting
* `Transformer` fall back to its default muxer.
*
* The guard had never fired. `Media3Engine$buildTransformer$3` -- the `requireNotNull` message
* lambda -- was four lines and four branches at 0%, which is to say the entire repair for a defect
* the codebase went to the trouble of writing down was untested. Weakening it would restore that
* bug silently, because the wrong output is a *playable file with the wrong container*, not a crash.
*
* Same harness and same two disciplines as [Media3EngineEmptyCompositionTest]: assert the plan
* really is the one the test needs before driving the engine, and rule out
* `CancellationException` so an unresumed continuation cannot read as a pass.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class Media3MuxerGuardTest {
@Test
fun `a container Media3 cannot mux fails the job rather than silently writing MP4`() {
val context = RuntimeEnvironment.getApplication()
val engine = Media3Engine(context)
val request = ConversionRequest(
spec = OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.OPUS),
probe = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MP4),
)
// The premise, asserted rather than assumed -- three separate ways this test could pass
// over a path it never entered.
val plan = CopyPlanner.plan(request.spec, request.probe)
assertEquals("the plan has to still be WebM by the time the engine sees it", Container.WEBM, plan.container)
assertNull("...and Media3 really has no muxer for it", Media3Muxers.factoryFor(plan.container))
// Not the empty-composition refusal, which fires earlier and is a different test's subject.
assertNotEquals(VideoPlan.Drop, plan.video)
assertNotEquals(AudioPlan.Drop, plan.audio)
val failure = try {
runCatching {
runBlocking {
withTimeout(TIMEOUT_MS) {
engine.transcode(Uri.parse("file:///dev/null"), File(context.cacheDir, "guard.webm"), request) {
}
}
}
}.exceptionOrNull()
} finally {
engine.close()
}
assertFalse(
"the continuation was never resumed -- the refusal escaped instead of failing the job: $failure",
failure is CancellationException,
)
// Type *and* message, and the message half is the load-bearing one. Replacing the
// requireNotNull with a fallback factory does not make the export succeed here: it lets it
// run on and fail some other way, which a bare type assertion would happily accept.
assertTrue("expected the muxer guard to refuse the job, got $failure", failure is IllegalArgumentException)
assertTrue(
"the refusal has to name the container it could not mux, got: ${failure?.message}",
failure?.message.orEmpty().contains("cannot mux") &&
failure?.message.orEmpty().contains(Container.WEBM.name),
)
}
private companion object {
/** Nothing is decoded or muxed on this path -- the guard refuses before any of that. */
const val TIMEOUT_MS = 10_000L
}
}
@@ -1,166 +0,0 @@
package org.libremediaconverter.convert
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.InputKind
import org.libremediaconverter.model.InputProbe
/**
* Which of the two probes wins, when they disagree.
*
* [MediaProbe.probe] runs `MediaExtractor` and FFprobe independently and then merges the two, and
* every rule in that merge is a decision. None of them had a test, for a reason that is structural
* rather than an oversight: `RemuxTest` drives the whole thing on a device against committed
* fixtures, but only ever with **one probe answering and the other agreeing or also failing**.
* Nothing on any source set can arrange for a real extractor and a real FFprobe to disagree, so
* every elvis in the merge was taken in one direction and never the other.
*
* Cutting `merge` out of `probe` is what makes the question askable. Both halves of its signature
* had to become `internal` for that -- `Extracted` already was, with a KDoc giving this exact
* reason; `FFprobeInfo` simply never got the same treatment.
*/
class MediaProbeMergeTest {
/**
* The rule with the loudest failure mode, and `isImageFormat`'s own KDoc names it: a false
* positive here "makes the source-info card describe a video as an image". So the image verdict
* has to beat a real video codec from the extractor, and the ordering that makes it do so is
* the first arm of `classify` rather than anything a reader would infer from the fields.
*/
@Test
fun `an image verdict from FFprobe beats a video codec from the extractor`() {
val merged = MediaProbe.merge(
extracted = extracted(video = "h264"),
info = info(video = "mjpeg", isImage = true),
)
assertEquals(InputKind.IMAGE, merged.kind)
}
@Test
fun `the extractor wins on codecs, because it is the view the router will act on`() {
val merged = MediaProbe.merge(
extracted = extracted(video = "h264", audio = "aac"),
info = info(video = "hevc", audio = "mp3"),
)
assertEquals("h264", merged.videoCodec)
assertEquals("aac", merged.audioCodec)
}
@Test
fun `FFprobe answers for a file the extractor could not open`() {
val merged = MediaProbe.merge(extracted = null, info = info(video = "vp9", audio = "opus"))
assertEquals("vp9", merged.videoCodec)
assertEquals("opus", merged.audioCodec)
assertEquals(InputKind.VIDEO, merged.kind)
}
@Test
fun `the extractor answers for a file FFprobe could not read`() {
val merged = MediaProbe.merge(extracted = extracted(video = "h264", audio = "aac"), info = null)
assertEquals("h264", merged.videoCodec)
assertEquals("aac", merged.audioCodec)
assertNull("only FFprobe can name the container, so it stays unknown here", merged.container)
}
/**
* The larger of the two, not the first non-zero.
*
* Either probe can report zero for a file the other times correctly, and a zero duration makes
* the FFmpeg progress percentage undefined -- `FFmpegEngine` divides by it. Both orderings are
* asserted because "take the extractor's" and "take the larger" agree in one direction and not
* the other, and only one of them is the rule.
*/
@Test
fun `duration is the longer of the two readings, whichever probe supplied it`() {
assertEquals(
5_000L,
MediaProbe.merge(extracted(duration = 0L), info(duration = 5_000L)).durationMs,
)
assertEquals(
5_000L,
MediaProbe.merge(extracted(duration = 5_000L), info(duration = 0L)).durationMs,
)
}
@Test
fun `dimensions come from the extractor, and from FFprobe only when it has none`() {
assertEquals(1920, MediaProbe.merge(extracted(width = 1920), info(width = 640)).width)
assertEquals(640, MediaProbe.merge(extracted = null, info = info(width = 640)).width)
assertEquals(0, MediaProbe.merge(extracted(width = 0), info(width = 0)).width)
}
@Test
fun `the container comes from FFprobe, which is the only probe that can name one`() {
val merged = MediaProbe.merge(extracted(video = "h264"), info(container = Container.MKV))
assertEquals(Container.MKV, merged.container)
}
@Test
fun `a file with audio and no video is audio-only, not unparseable`() {
val merged = MediaProbe.merge(extracted(video = null, audio = "mp3"), info = null)
assertEquals(InputKind.AUDIO_ONLY, merged.kind)
assertFalse(merged.hasVideo)
}
@Test
fun `a file neither probe could open is the one unreadable answer`() {
val merged = MediaProbe.merge(extracted = null, info = null)
assertEquals(MediaProbe.UNREADABLE, merged)
assertEquals(InputProbe.UNPARSEABLE, merged.videoCodec)
}
/**
* The arm the ticket was filed for: parsed, and carrying no stream either probe recognised.
*
* Distinct from "neither probe could open it" -- here the extractor opened the file happily and
* found nothing convertible, which is what a container holding only subtitles looks like. It
* has to reach the same [MediaProbe.UNREADABLE] answer, because the router keys off that and
* there is nothing here for Media3 to do either way.
*
* Its input was already being built elsewhere in the suite -- `MediaProbeTrackWalkTest` calls
* `extractedFrom(emptyList())` and gets exactly this -- and had simply never been handed to the
* merge.
*/
@Test
fun `a file that parsed but carries no recognised stream is unreadable too`() {
val merged = MediaProbe.merge(extracted = MediaProbe.extractedFrom(emptyList()), info = null)
assertEquals(InputKind.UNPARSEABLE, merged.kind)
assertEquals(MediaProbe.UNREADABLE, merged)
}
@Test
fun `hasVideo follows the codec that survived the merge, not either probe alone`() {
assertTrue(MediaProbe.merge(extracted(video = null), info(video = "vp9")).hasVideo)
assertFalse(MediaProbe.merge(extracted(video = null, audio = "aac"), info(video = null)).hasVideo)
}
private fun extracted(
video: String? = "h264",
audio: String? = "aac",
duration: Long = 1_000L,
width: Int = 1280,
height: Int = 720,
) = MediaProbe.Extracted(video, audio, duration, width, height)
private fun info(
container: Container? = null,
video: String? = "h264",
audio: String? = "aac",
duration: Long = 1_000L,
width: Int = 1280,
height: Int = 720,
isImage: Boolean = false,
) = MediaProbe.FFprobeInfo(container, video, audio, duration, width, height, isImage)
}
@@ -1,213 +0,0 @@
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()
}
@@ -232,12 +232,6 @@ 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))
@@ -1,70 +0,0 @@
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")
}
}
@@ -166,54 +166,6 @@ class FFmpegCommandBuilderTest {
assertPair(cmd(OutputFormat.OPUS), "-c:a", "libopus")
}
/**
* The arm most conversions actually take, and the only one in `audioArgs` with no test.
*
* `flac wav and opus select the right encoders` above covers the three named arms; MP3 has its
* own. AAC arrives through the `else`, so nothing named it and nothing pinned either half of
* what it emits -- neither `aac` nor `192k` appeared anywhere in this file. Both are shipped
* defaults: MP4 and M4A are the formats the picker offers first, so this is the audio
* every ordinary conversion gets.
*
* The bitrate is asserted as well as the encoder because it is the half a refactor is likelier
* to lose. An `-b:a` that quietly changed would not fail anything, would not look wrong in a
* command line, and would show up only as files that sound different from the ones the app
* produced last month.
*/
@Test
fun `aac is the default encoder, at the bitrate the app ships`() {
assertPair(cmd(OutputFormat.MP4_H264), "-c:a", "aac")
assertPair(cmd(OutputFormat.MP4_H264), "-b:a", "192k")
// Through the `else` rather than through a named arm, so an AAC branch added above it later
// has to keep answering the same way.
assertPair(cmd(OutputFormat.M4A_AAC), "-c:a", "aac")
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)
@@ -451,99 +451,6 @@ 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
@@ -28,7 +28,6 @@ class TagTableUniquenessTest {
fun `every tag constant has its own value`() {
val tags = tagsIn(
TestTags::class.java,
TestTags.Shell::class.java,
TestTags.Converter::class.java,
TestTags.Join::class.java,
)
@@ -1,237 +0,0 @@
package org.libremediaconverter.work
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.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConcatJoiner
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.StagingNames
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.ffmpeg.ConcatEngine
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.model.OutputFormat
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.UUID
/**
* What a join does when the engine fails partway.
*
* Everything past `ConcatWorker`'s `setForeground` was untested on **every** source set, and the
* repo had already measured the cost: `PerJobStagingTest`'s KDoc records that reverting
* `ConcatWorker` to a constant staging name left all 257 tests green, because nothing in the JVM
* suite can get past a `ConcatEngine` constructed in place. `RefusedJobTest` says the same from the
* other side -- "the next thing past the count guard is `ConcatEngine`, which is native".
* `ConcatEngineTest` on a device tests the engine directly, bypassing the worker, and
* `ConcatWorkerTest` covers only the too-few-inputs guard and the happy path.
*
* `ConversionDependencies.concat` is the seam that closes it, added here to sit beside the
* `.hardware` and `.software` that `ConversionWorker` has had all along -- the asymmetry between the
* two workers was the whole reason one of them had a tested failure path and the other did not.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ConcatFailureTest {
private lateinit var app: Application
private lateinit var publisher: AlwaysRoomPublisher
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
publisher = AlwaysRoomPublisher(app)
ConversionDependencies.publisher = { publisher }
installTestWorkManager(app, Data.EMPTY)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `a join whose engine fails reports the engine's own reason`() {
ConversionDependencies.concat = { FailingJoiner { error(DEMUXER_MESSAGE) } }
val result = runBlocking { joinWorker().doWork() }
assertEquals(
ListenableWorker.Result.failure(workDataOf(ConcatWorker.KEY_ERROR to DEMUXER_MESSAGE)),
result,
)
}
/**
* A failure carrying no message at all, which Kotlin and Java both allow and FFmpegKit's
* wrappers can produce.
*
* Without the fallback the user is shown an empty error, and `JoinViewModel` cannot tell that
* from a job that reported nothing -- the two would be one blank screen with different causes.
*/
@Test
fun `a failure with no message of its own still says something`() {
ConversionDependencies.concat = { FailingJoiner { throw RuntimeException() } }
val result = runBlocking { joinWorker().doWork() }
assertEquals(
ListenableWorker.Result.failure(
workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.GENERIC_FAILURE_MESSAGE),
),
result,
)
}
/**
* The staged file is deleted on the way out.
*
* The joiner writes before it fails, exactly as `PartialThenFailingTranscoder` does on the
* conversion side: a stub that only threw would let a missing `staged.delete()` pass. What it
* costs to lose is a full-size partial per failed join, sitting in cache until the sweep is old
* enough to be sure nobody is coming back for it.
*
* Asserted against the file the joiner was actually handed rather than by scanning the staging
* directory for a name. The first draft did scan, for a `"join-"` prefix that
* `StagingNames.forJob` does not produce -- it names files `<jobId>.<ext>` -- so the assertion
* was trivially true and the mutation walked straight through it.
*/
@Test
fun `a failed join leaves nothing behind in staging`() {
val joiner = FailingJoiner { error(DEMUXER_MESSAGE) }
ConversionDependencies.concat = { joiner }
runBlocking { joinWorker().doWork() }
val staged = requireNotNull(joiner.lastOutput) { "the joiner never ran, so this proves nothing" }
assertEquals(
"the fixture has to write before it fails, or the delete is unobservable",
PARTIAL_BYTES,
joiner.bytesWritten,
)
assertFalse("a failed join must not leave its partial behind: $staged", staged.exists())
}
/**
* The success path, and the staging name #159's fixture and `PerJobStagingTest` both care about.
*
* Worth its place rather than a happy-path formality: `PerJobStagingTest`'s KDoc records that
* **reverting `ConcatWorker` to a constant staging name left all 257 tests green**, because
* nothing could reach the line that names the file. This is the test that was missing when that
* was written -- the join's output `Data` had never been read by anything on the JVM.
*
* The staged path is asserted to carry the job id, not a constant: two joins of the same format
* sharing one name is the defect, and `ConcatEngine`'s list file collided harder still.
*/
@Test
fun `a join that works reports its own staged file, strategy and name`() {
val joiner = SucceedingJoiner()
ConversionDependencies.concat = { joiner }
val result = runBlocking { joinWorker().doWork() }
assertTrue("got $result", result is ListenableWorker.Result.Success)
val data = (result as ListenableWorker.Result.Success).outputData
assertEquals(
"the staged file has to be this job's, not a name every join shares",
File(publisherStagingDir(), StagingNames.forJob(JOB_ID, OutputFormat.MP4_H264.extension)).absolutePath,
data.getString(ConcatWorker.KEY_OUTPUT_PATH),
)
assertEquals(ConcatStrategy.STREAM_COPY.name, data.getString(ConcatWorker.KEY_STRATEGY))
assertEquals(OutputFormat.MP4_H264.mimeType, data.getString(ConcatWorker.KEY_MIME_TYPE))
assertTrue(
"the save dialog needs a name with the right extension, got ${data.getString(
ConcatWorker.KEY_SUGGESTED_NAME,
)}",
data.getString(ConcatWorker.KEY_SUGGESTED_NAME).orEmpty().endsWith(".${OutputFormat.MP4_H264.extension}"),
)
}
/**
* The arm beside the count guard: no URI array at all.
*
* Covered today only by `UnopenableUriTest` on a device, although it runs before staging and
* before any native code. It is the exact sibling of `RefusedJobTest`'s ConversionWorker twin,
* and it belongs on the JVM with it -- a device test for a branch that needs no device is a
* slower test that reports later.
*/
@Test
fun `a join with no input array at all is refused with a message`() {
val result = runBlocking {
TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name),
runAttemptCount = 0,
).setId(JOB_ID).build().doWork()
}
assertEquals(
ListenableWorker.Result.failure(workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.NO_INPUTS_MESSAGE)),
result,
)
}
private fun publisherStagingDir(): File? = publisher.createStagingFile("probe").parentFile
private fun joinWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(
ConcatWorker.KEY_INPUT_URIS to arrayOf(FIRST.toString(), SECOND.toString()),
ConcatWorker.KEY_TOTAL_BYTES to TOTAL_BYTES,
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
),
runAttemptCount = 0,
).setId(JOB_ID).build()
private companion object {
val FIRST: Uri = Uri.parse("file:///tmp/one.mp4")
val SECOND: Uri = Uri.parse("file:///tmp/two.mp4")
const val TOTAL_BYTES = 2048L
const val DEMUXER_MESSAGE = "the demuxer rejected the input list"
const val PARTIAL_BYTES = 2048
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000b")
}
}
/** A joiner that writes something and then fails, so a missing `staged.delete()` cannot pass. */
private class FailingJoiner(private val failure: () -> Nothing) : ConcatJoiner {
/** The handle the worker created, kept so a test can ask whether it survived the failure. */
var lastOutput: File? = null
var bytesWritten = 0
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
lastOutput = output
output.writeBytes(ByteArray(PARTIAL_BYTES))
bytesWritten = PARTIAL_BYTES
failure()
}
private companion object {
const val PARTIAL_BYTES = 2048
}
}
/** The joiner that finishes, so the success path and the output `Data` can be read on the JVM. */
private class SucceedingJoiner : ConcatJoiner {
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
output.writeBytes(ByteArray(OUTPUT_BYTES))
return ConcatEngine.Result(ConcatStrategy.STREAM_COPY, output)
}
private companion object {
const val OUTPUT_BYTES = 4096
}
}
@@ -1,88 +0,0 @@
package org.libremediaconverter.work
import android.content.pm.ServiceInfo
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
/**
* [ConversionForegroundType.current] answers differently on each of the three API regimes, and
* until this file only one of them was ever executed.
*
* `app/src/test/resources/robolectric.properties` pins the whole JVM suite to `sdk=36`, so every
* Robolectric test that reaches a `ForegroundInfo` takes the `mediaProcessing` arm and no other.
* The 33 and 34 arms were cold: 3 lines and 3 of 4 branches, measured on `main` at `d354f64`.
*
* **The instrumented test is not a substitute, and the reason is specific.**
* `ConversionWorkerTest.foregroundTypeMatchesTheRunningApiLevel` asserts against whichever API the
* leg happens to be — one arm per leg, never the other two — and the legs that would cover 33 and
* 34 are the ones issue #122 wedges. `docs/coverage-read-findings.md` records an API 33 run that
* reported `received: 60` and `failed: unknown`: the regime *was* exercised, and that leg could
* not have said so if it had broken. Four `@Config` classes here pin all three arms
* deterministically, in the same `./gradlew` invocation as everything else.
*
* `minSdk` is 33, so none of these is dead code — each is a device someone is running the app on.
*
* **SDK 35 is in the list for the boundary, not for the answer.** It shares its answer with 36,
* which would make it look redundant. It is not: relaxing `>= VANILLA_ICE_CREAM` to `>` is invisible
* at every level except exactly 35, so without this class that mutation survives the suite.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [33])
class ForegroundTypeApi33Test {
/**
* Zero rather than a named constant because there is no constant to name: API 33 does not
* require a type, and `mediaProcessing` does not exist here to pass. `ForegroundInfo` reads 0
* as "no type at all", which is what this regime wants.
*/
@Test
fun `api 33 asks for no foreground service type`() {
assertEquals(0, ConversionForegroundType.current())
}
}
/**
* API 34 makes a type mandatory and still has no `mediaProcessing`, so `dataSync` is the only
* sensible fit. See [ForegroundTypeApi33Test] for why this file exists.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34])
class ForegroundTypeApi34Test {
@Test
fun `api 34 falls back to dataSync, the only type that fits`() {
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC, ConversionForegroundType.current())
}
}
/**
* The first level with `mediaProcessing`, and therefore the one that tells `>=` from `>`.
* See [ForegroundTypeApi33Test].
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [35])
class ForegroundTypeApi35Test {
@Test
fun `api 35 is the first level that takes mediaProcessing`() {
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING, ConversionForegroundType.current())
}
}
/**
* The level the rest of the suite runs at, asserted here rather than assumed — it is the one arm
* that was already covered, and leaving it out would make this file look like it is about the old
* levels rather than about all three regimes. See [ForegroundTypeApi33Test].
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [36])
class ForegroundTypeApi36Test {
@Test
fun `api 36 keeps mediaProcessing`() {
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING, ConversionForegroundType.current())
}
}
@@ -1,226 +0,0 @@
package org.libremediaconverter.work
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 kotlinx.coroutines.CancellationException
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertThrows
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.HardwareTranscoder
import org.libremediaconverter.convert.SoftwareTranscoder
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.DeviceCodecs
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.UUID
/**
* What happens when the hardware engine does not finish the job.
*
* `runMedia3OrFallBack` was eleven lines at 0% on the JVM and `isCancellation` had never been
* called by any unit test at all. Its own KDoc calls the fallback the protection against vendor
* hardware encoders that "cannot be tested for correctness", so it is the branch most likely to
* matter on a device nobody here owns — and it was reachable the whole time through
* `ConversionDependencies.hardware`, which no unit test had ever used.
*
* The sharp one is cancellation. `runMedia3OrFallBack` catches `Throwable`, so without the
* `isCancellation` re-throw a user cancelling a hardware transcode would have the app quietly
* start a *second* conversion in software — the one thing cancelling is supposed to prevent.
*
* `ForcedFailureTest` covers the failure half on a device. It does not cover the cancellation half,
* and this host cannot run it either way.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class HardwareFallbackTest {
private lateinit var app: Application
private lateinit var hardware: RecordingHardwareTranscoder
private lateinit var software: RecordingSoftwareTranscoder
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
hardware = RecordingHardwareTranscoder()
software = RecordingSoftwareTranscoder()
ConversionDependencies.publisher = { AlwaysRoomPublisher(app) }
ConversionDependencies.hardware = { hardware }
ConversionDependencies.software = { software }
// A probe with real codecs, not the default: `InputProbe()` reports UNPARSEABLE, which
// PERMISSIVE.canDecode refuses, and the router would send every job here straight to
// FFmpeg without any of these tests mentioning why.
ConversionDependencies.probe = { _, _ -> H264_SOURCE }
ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }
installTestWorkManager(app, Data.EMPTY)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `a hardware failure runs the job again in software, on a clean staging file`() {
hardware.failWith = { error("the vendor encoder produced nothing usable") }
val result = runBlocking { worker().doWork() }
assertTrue("the job should still succeed, got $result", result is ListenableWorker.Result.Success)
assertEquals("the hardware engine gets exactly one attempt", 1, hardware.attempts)
assertEquals("and the job then goes to software", 1, software.attempts)
// The `staged.delete()` between the two, asserted where it is observable: FFmpeg must not
// find a half-written hardware output sitting at the path it is about to write.
assertFalse(
"the partial hardware output must be gone before FFmpeg starts",
software.outputExistedOnEntry,
)
assertEquals("the hardware engine is closed either way", 1, hardware.closes)
}
@Test
fun `a cancelled hardware transcode is not quietly retried in software`() {
hardware.failWith = { throw CancellationException("the user pressed Cancel") }
assertThrows(CancellationException::class.java) { runBlocking { worker().doWork() } }
assertEquals("the hardware engine ran", 1, hardware.attempts)
assertEquals(
"cancelling must not start a second conversion -- that is the whole point of cancelling",
0,
software.attempts,
)
assertEquals("and the engine is still closed on the way out", 1, hardware.closes)
}
@Test
fun `a hardware transcode that works never reaches the software engine`() {
val result = runBlocking { worker().doWork() }
assertTrue("got $result", result is ListenableWorker.Result.Success)
assertEquals(1, hardware.attempts)
assertEquals("the fallback is a fallback, not a second pass", 0, software.attempts)
assertEquals(1, hardware.closes)
}
/**
* #169: the display-name fallback, which reaches further than the notification title.
*
* `inputData.getString(KEY_DISPLAY_NAME) ?: "input"` had never taken its right-hand side. The
* value is not only the foreground notification's title: it feeds `outputNameFor`, so it is
* also the filename offered in the user's save dialog. A job enqueued by an older build, or
* built by hand, carries no such key.
*/
@Test
fun `a job that names no input file still suggests an output name`() {
val result = runBlocking { worker(displayName = null).doWork() }
assertTrue("got $result", result is ListenableWorker.Result.Success)
val suggested = (result as ListenableWorker.Result.Success)
.outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME)
assertTrue(
"expected a name built from the fallback, got $suggested",
suggested.orEmpty().startsWith("input"),
)
}
private fun worker(displayName: String? = DISPLAY_NAME): ConversionWorker {
val spec = OutputFormat.MP4_H265.spec
val entries = buildMap<String, Any> {
put(ConversionWorker.KEY_INPUT_URI, INPUT.toString())
displayName?.let { put(ConversionWorker.KEY_DISPLAY_NAME, it) }
put(ConversionWorker.KEY_SIZE_BYTES, INPUT_BYTES)
put(ConversionWorker.KEY_CONTAINER, spec.container.name)
put(ConversionWorker.KEY_VIDEO_CODEC, spec.videoCodec.name)
put(ConversionWorker.KEY_AUDIO_CODEC, spec.audioCodec.name)
// AUTO rather than FORCE_SOFTWARE, which is what every other worker test uses and is
// exactly why this path had no coverage: forcing software never enters the function.
put(ConversionWorker.KEY_ENGINE_PREFERENCE, EnginePreference.AUTO.name)
}
return TestListenableWorkerBuilder<ConversionWorker>(
context = app,
inputData = Data.Builder().putAll(entries).build(),
runAttemptCount = 0,
).setId(JOB_ID).build()
}
private companion object {
val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4")
const val DISPLAY_NAME = "holiday.mp4"
const val INPUT_BYTES = 1024L
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000009")
val H264_SOURCE = InputProbe(
videoCodec = "h264",
audioCodec = "aac",
container = Container.MP4,
durationMs = 1_000,
)
}
}
/**
* A hardware engine that writes something before it fails, and remembers being closed.
*
* Writing first is the point, exactly as it is for `PartialThenFailingTranscoder`: an engine that
* only threw would let a missing `staged.delete()` pass unnoticed.
*/
@UnstableApi
private class RecordingHardwareTranscoder : HardwareTranscoder {
var attempts = 0
var closes = 0
var failWith: (() -> Unit)? = null
override suspend fun transcode(input: Uri, output: File, request: ConversionRequest, onProgress: (Int) -> Unit) {
attempts++
output.writeBytes(ByteArray(PARTIAL_BYTES))
failWith?.invoke()
}
override fun close() {
closes++
}
private companion object {
const val PARTIAL_BYTES = 2048
}
}
/** The software engine, recording whether the hardware attempt's leftovers were cleared first. */
private class RecordingSoftwareTranscoder : SoftwareTranscoder {
var attempts = 0
var outputExistedOnEntry = false
override suspend fun run(
request: ConversionRequest,
inputPath: String,
output: File,
durationMs: Long,
onProgress: (Int) -> Unit,
) {
attempts++
outputExistedOnEntry = output.exists()
output.writeBytes(ByteArray(OUTPUT_BYTES))
}
private companion object {
const val OUTPUT_BYTES = 512
}
}
@@ -16,7 +16,6 @@ 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
@@ -126,39 +125,6 @@ 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<JobSnapshot> = runBlocking {
workManager.jobSnapshots(
tag = ConversionWorker::class.java.name,
@@ -186,11 +152,6 @@ 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<ConversionWorker>().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
@@ -1,89 +0,0 @@
package org.libremediaconverter.work
import android.app.Notification
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.installTestWorkManager
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.util.UUID
/**
* The two things a progress notification can say, and that they are not the same thing.
*
* An assertion gap rather than a coverage one, and the distinction is the reason this file exists.
* JaCoCo is green on `build`'s `if (indeterminate)`, because `ProgressNotificationTest` drives it
* through a real worker -- but that test reads only the notification id and
* `Notification.EXTRA_PROGRESS`. **Nothing had ever read the text.** Swapping the two branches, or
* collapsing them into one string, passed the entire suite.
*
* What it costs to get wrong is small and constant: a conversion that has been running for four
* minutes still saying "Preparing", or one that has not started reporting yet claiming 0%. Neither
* is a crash, and neither would be found by anything else here -- which is exactly the kind of
* thing that survives for a long time.
*
* Nothing else in the suite constructs [ConversionNotifications] directly.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class NotificationProgressTextTest {
/**
* `build` reaches `WorkManager.getInstance` for the Cancel action's PendingIntent, so the
* notification cannot be built at all without one. That coupling is why nothing had ever
* constructed this class directly and read what it produced.
*/
@Before
fun setUp() {
installTestWorkManager(RuntimeEnvironment.getApplication(), Data.EMPTY)
}
@Test
fun `an indeterminate notification says something different from a measured one`() {
val context = RuntimeEnvironment.getApplication()
val notifications = ConversionNotifications(context)
val preparing = notifications.build(JOB_ID, TITLE, percent = 0, indeterminate = true).text()
val measured = notifications.build(JOB_ID, TITLE, percent = 42, indeterminate = false).text()
assertNotEquals(
"the two states have to read differently, or the text says nothing at all",
preparing,
measured,
)
assertTrue(
"a measured notification has to carry its percentage, got \"$measured\"",
measured.contains("42"),
)
assertTrue(
"an indeterminate one must not invent one, got \"$preparing\"",
!preparing.contains("42") && !preparing.contains("0"),
)
}
/**
* The title is the caller's, not the builder's -- it is the file the user picked, and it is what
* tells two simultaneous conversions apart in the shade.
*/
@Test
fun `the notification is titled with the file it is converting`() {
val context = RuntimeEnvironment.getApplication()
val built = ConversionNotifications(context).build(JOB_ID, TITLE, percent = 10)
assertEquals(TITLE, built.extras.getString(Notification.EXTRA_TITLE))
}
private fun Notification.text(): String = extras.getString(Notification.EXTRA_TEXT).orEmpty()
private companion object {
const val TITLE = "holiday.mp4"
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000a")
}
}
@@ -21,10 +21,8 @@ import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.HardwareTranscoder
import org.libremediaconverter.convert.SoftwareTranscoder
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.DeviceCodecs
import org.libremediaconverter.model.EnginePreference
@@ -131,40 +129,6 @@ class ProgressNotificationTest {
)
}
/**
* The same plumbing on the engine most conversions actually use, which had none.
*
* `ConversionWorker.kt:208-210` is a second `onProgress` lambda at a second call site — the one
* handed to `engine.transcode` — and it reported `ci == 0`. Every test above drives the FFmpeg
* path; `HardwareFallbackTest` reaches `runMedia3OrFallBack` but its recording transcoder
* records the call and never invokes the callback it was given. So the two engines' progress
* wiring was one tested and one not, and the untested one is the default: `ConversionRouter`
* sends everything it can to Media3.
*
* `AUTO` with a real H.264 probe, because `FORCE_SOFTWARE` is precisely what keeps the other
* tests out of this branch. The probe and the permissive codec profile are what let the router
* choose Media3 at all — `InputProbe()` reports `UNPARSEABLE`, which routes straight to FFmpeg.
*
* Asserted on the *percentage*, not merely on an update having happened: `publishProgress`
* takes a display name and a percent, and replacing the percent with a constant compiles.
*/
@Test
fun `progress from the hardware engine reaches WorkManager the same way FFmpeg's does`() {
ConversionDependencies.probe = { _, _ -> H264_SOURCE }
val reporting = ReportingHardwareTranscoder { onProgress -> onProgress(PERCENT) }
ConversionDependencies.hardware = { reporting }
runBlocking { workerReporting(EnginePreference.AUTO) { }.doWork() }
assertEquals("the job must have gone to the hardware engine", 1, reporting.attempts)
val progressUpdates = updater.infos.drop(1)
assertEquals("one throttled progress update expected", 1, progressUpdates.size)
assertEquals(
PERCENT,
progressUpdates.single().notification.extras.getInt(Notification.EXTRA_PROGRESS),
)
}
/**
* A worker routed to the software engine, whose engine is [report] and a written output.
*
@@ -173,10 +137,7 @@ class ProgressNotificationTest {
* bridge, which is native. [report] is handed the worker's own progress callback, and runs with
* the worker as its receiver so a test can stop it mid-transcode.
*/
private fun workerReporting(
enginePreference: EnginePreference = EnginePreference.FORCE_SOFTWARE,
report: ConversionWorker.((Int) -> Unit) -> Unit,
): ConversionWorker {
private fun workerReporting(report: ConversionWorker.((Int) -> Unit) -> Unit): ConversionWorker {
val worker = TestListenableWorkerBuilder<ConversionWorker>(
context = app,
inputData = workDataOf(
@@ -186,7 +147,7 @@ class ProgressNotificationTest {
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.name,
ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name,
),
runAttemptCount = 0,
).setId(JOB_ID)
@@ -210,17 +171,6 @@ class ProgressNotificationTest {
const val TICKS = 50
val SPEC = OutputFormat.MP4_H265.spec
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000021")
/**
* A probe the router can actually route. `InputProbe()` reports `UNPARSEABLE`, which
* `PERMISSIVE.canDecode` refuses, so every job would reach FFmpeg with no test saying why.
*/
val H264_SOURCE = InputProbe(
videoCodec = "h264",
audioCodec = "aac",
container = Container.MP4,
durationMs = 1_000,
)
}
}
@@ -261,22 +211,3 @@ private class ReportingTranscoder(private val report: ((Int) -> Unit) -> Unit) :
const val OUTPUT_BYTES = 512
}
}
/** A hardware engine that reports whatever [report] wants reported, then writes an output. */
@UnstableApi
private class ReportingHardwareTranscoder(private val report: ((Int) -> Unit) -> Unit) : HardwareTranscoder {
var attempts = 0
override suspend fun transcode(input: Uri, output: File, request: ConversionRequest, onProgress: (Int) -> Unit) {
attempts++
report(onProgress)
output.writeBytes(ByteArray(OUTPUT_BYTES))
}
override fun close() = Unit
private companion object {
const val OUTPUT_BYTES = 16
}
}
+17 -233
View File
@@ -1,27 +1,22 @@
# Coverage-read findings
**Status:** ten findings, none fixed, none urgent. F1-F4 came from the 2026-08-26 read; F5 was added
on 2026-08-27 while decomposing #132; **F6-F10 were added on 2026-09-02 from the wave-4 read**. Every
entry here is a *code* observation — something a test would document rather than repair. The test
gaps found in the same reads are tickets, not entries here; see [Not covered here](#not-covered-here).
**Scope:** what a JaCoCo read turned up that writing a test would not fix. This is a survey, not a
work order. Acting on any entry is a separate decision and would be its own commit.
**Last verified:** `main` at `54ca2dd`, 2026-09-02. Coverage measured that day with
`./gradlew :app:jacocoTestReport`: **92.8% line (2183/2352), 81.3% branch (1091/1342)**, against
**584 JVM tests in 87 classes**, matching what `CLAUDE.md` quotes.
The wave-4 read that produced F6-F10 also produced twelve test tickets, **#192-#203**, plus **#204**
for four candidates whose cost was not obviously worth paying. The split between them is the same one
this document has always drawn: a ticket is where a test goes, an entry here is where a test would not
help.
**Status:** five findings, none fixed, none urgent. F5 was added on 2026-08-27, found while decomposing #132 into children — it had been listed there as a test gap, and is not one. Every entry here is a *code* observation —
something a test would document rather than repair. The test gaps found in the same read are
tickets #132 and #133, not entries here; see [Not covered here](#not-covered-here).
**Scope:** what a JaCoCo read on 2026-08-26 turned up that writing a test would not fix. This is
a survey, not a work order. Acting on any entry is a separate decision and would be its own commit.
**Last verified:** `main` at `dc8b7c3`, 2026-08-26. Coverage re-measured that day with
`./gradlew :app:jacocoTestReport`: **84.9% line (1971/2321), 63.8% branch (900/1410)**, against
**456 JVM tests in 68 classes**. `CLAUDE.md` quotes 454 in 67 from four hours earlier; the
percentages are unchanged, so no figure there is stale.
## Why this document is separate from `defect-audit.md`
`defect-audit.md` is the record of the 2026-08-22 defect sweep: sixteen entries, each a thing that
is *wrong at runtime*. Nothing here is wrong at runtime today. These are arms that cannot be
reached, accessors nobody calls, and two KDocs that contradict the code beside them — the category
`defect-audit.md` calls **latent**, plus several that are not defects at all and are recorded so the
next coverage read does not re-file them.
reached, accessors nobody calls, and one KDoc that contradicts the code beside it — the category
`defect-audit.md` calls **latent**, plus one that is not a defect at all and is recorded so the
next coverage read does not re-file it.
They are here rather than in that document because folding them in would inflate a sixteen-entry
audit whose status metadata has already gone stale once, and because they share a provenance:
@@ -29,7 +24,7 @@ every one fell out of reading a coverage report, and every one is the kind of th
report is *good* at surfacing and a test is bad at fixing. F5 is the clearest case — it was filed
as a test gap first, and only stopped being one when someone went looking for its callers.
Entry ids are `F1`–`F10` so they cannot be confused with `defect-audit.md`'s `D1`–`D16`.
Entry ids are `F1`–`F5` so they cannot be confused with `defect-audit.md`'s `D1`–`D16`.
## How to read the confidence labels
@@ -267,179 +262,6 @@ no way to make it happen now.
---
## F6 — Four more arms that cannot be reached, and one KDoc among them that is false
**Severity: low · Confirmed by inspection · F4's family, found in the wave-4 read**
```
app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt:178-179
app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt:221
app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt:297
app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt:324, :340
```
Four sites that a coverage report flags and that no test can reach. Each is recorded with the
upstream guard that makes it unreachable, because that guard is what would have to change first.
- **`ConversionRouter:178-179`** — the missed branch is `orEmpty()`'s absent-key arm on
`MEDIA3_MUXABLE_VIDEO[plan.container]`. `MEDIA3_CONTAINERS` is `setOf(MP4)` and `route()` returns at
`:104` for anything else, so `media3CanMux` only ever sees MP4, which both maps key. Same function
as F4's second pair, one line below it.
- **`ConversionRouter:221`** — `DeviceCodecs.PERMISSIVE.canDecode` returning **false** for
`InputProbe.UNPARSEABLE`. `PERMISSIVE` has no production caller at all (tests only), and the
router's one `canDecode` call at `:128` is already preceded by `:117` returning FFMPEG for
`UNPARSEABLE`. **Its KDoc at `:214-217` is false as written:**
> That exception matters: a device double that claims it can decode an unparseable file would let
> the router send a doomed job to Media3.
It would not — `:117` already caught it. This is F2's shape: a comment that describes a hazard the
code upstream has removed. Correcting it is a one-line change and should not be bundled with
anything.
- **`ContainerCapabilities:297`** — `if (container == GIF || container == IMAGE_SEQUENCE) return null`
in `repair`. `repair`'s only caller is `suggestions` (`:281`); `validate` returns at `:121` for
`isImageOutput` (which is exactly GIF ∥ IMAGE_SEQUENCE) before `suggestions` is reached, and
`firstContainerHolding` filters on `CARRIES_VIDEO`, which is empty for both.
- **`ContainerCapabilities:324` and `:340`** — the `else ->` arms themselves are exercised; what is
missed is the elvis tail, `firstOrNull() ?: VideoCodec.NONE` / `?: AudioCodec.NONE`. Reaching it
needs a container with no encodable codec on that axis. Audio-only containers return early at
`:307`, and the only containers with an empty audio set are GIF and IMAGE_SEQUENCE, excluded at
`:297` above.
**Recorded so the next read does not re-file them.** F4's rule applies unchanged: a second line of
defence that can be provoked is not a second line of defence, and widening a private function to make
one reachable buys a test that asserts a fallback fires when called in a way production cannot call
it.
---
## F7 — `probeWithExtractor`'s catch is unreachable for the same measured reason `probeForConcat`'s is
**Severity: n/a · No action · completes a measurement already on record**
```
app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt:180-182
```
```kotlin
} catch (e: Exception) {
Log.i(TAG, "Platform extractor could not read $uri.", e)
null
}
```
`CLAUDE.md` records the measurement for the *other* extractor site: Robolectric's `MediaExtractor`
never throws from `setDataSource`, checked across an unregistered `content://` authority, a missing
`file://`, a file of garbage bytes and an `http://` URL — all four returned with `trackCount = 0`.
`probeWithExtractor` calls the same overload, three lines apart in the same file, and the measurement
covers it identically. It was simply not written down for this site, so a future read would re-derive
it. It stays device-only, alongside `probeForConcat`'s.
**Two neighbouring line counts are artifacts of this, not separate gaps.** `MediaProbe:184` and
`:331` each report 27 missed instructions and are the `finally` block's synthetic exception-path copy
— JaCoCo duplicates a `finally` per exit path, and the exceptional one is unreachable for the reason
above. Do not read them as a third and fourth site.
---
## F8 — Three more dead members, and six unused defaults
**Severity: low · Confirmed by inspection · F3's family**
```
app/src/main/java/org/libremediaconverter/model/CopyPlanner.kt:28 ConversionPlan.hasVideo
app/src/main/java/org/libremediaconverter/codec/AndroidDeviceCodecs.kt:39 hardwareEncoders()
app/src/main/java/org/libremediaconverter/ffmpeg/ConcatEngine.kt:30 Result.output
app/src/main/java/org/libremediaconverter/convert/Transcoders.kt:28, :29, :40, :61
app/src/main/java/org/libremediaconverter/work/Reattachment.kt:28, :30
```
- **`ConversionPlan.hasVideo`** — zero callers in `main`, `test` or `androidTest`. Every `hasVideo`
hit in the tree is `InputProbe.hasVideo`, `OutputSpec.hasVideo` or `Container.extensionFor(hasVideo)`,
which are different properties on different types. A test asserting
`plan.hasVideo == (plan.video != VideoPlan.Drop)` is vacuous by construction.
- **`AndroidDeviceCodecs.hardwareEncoders()`** — its only caller is `RealMediaBenchmark`, in
`androidTest`. Production reads capabilities through `DeviceCodecs`, never the raw set.
- **`ConcatEngine.Result.output`** — `ConcatWorker` reads `result.strategy` and uses the `staged`
file it passed in, never `.output`.
- **`Transcoders.kt`'s default arguments** — `request` and `onProgress` on
`HardwareTranscoder.transcode` (`:28`, `:29`), `onProgress` on `SoftwareTranscoder.run` (`:40`),
and `format` on `ConcatJoiner.join` (`:61`). All three production call sites
(`ConversionWorker.kt:208`, `:234`, `ConcatWorker.kt:79`) pass every argument, so the synthesised
`$default` bridges and `$DefaultImpls` copies are never entered. The
`request: ConversionRequest = ConversionRequest(OutputFormat.MP4_H265.spec)` default is the one
worth a second look: nothing anywhere omits it, so an interface silently promises H.265 to a
caller that does not exist.
- **`JobSnapshot`'s `outputModifiedAt` and `tags` defaults** — `JobSnapshots.kt:32-42` passes all
seven fields, so the synthesised `$default` constructor (20 missed instructions at
`Reattachment.kt:14`) is never entered.
**Not a test gap, for F3's reason.** Delete them, or keep them and know they are unused; either is a
decision, and a test restating the compiler is not.
---
## F9 — Both workers' `getForegroundInfo` overrides are dead, and this is why
**Severity: n/a · No action · sharpens #88 rather than reopening it**
```
app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:342-346
app/src/main/java/org/libremediaconverter/work/ConcatWorker.kt:132-136
```
**#88 already closed on these**, after reading both and finding no decision worth a seam — the
correct call, and it stands. What #88 did not name is the reason they are cold in the first place,
which is stronger than "the JVM cannot reach them":
WorkManager calls `getForegroundInfoAsync()` **only for expedited work**. `ConversionWorker`'s own
KDoc says expedited is deliberately not used, and `grep -rn 'setExpedited\|OutOfQuotaPolicy' app/src`
returns nothing. So both overrides are dead in production today, not merely untested — a test would
assert the shape of something nothing invokes.
They are still correct to keep: `ForegroundInfo` is required by the `CoroutineWorker` contract and
`setForeground` is called explicitly elsewhere. **What would reopen this** is the same trigger #88
named — a `getForegroundInfo` that starts branching — plus one more: the day anything calls
`setExpedited`.
---
## F10 — Three arms that are reachable, uncovered, and cannot be made to bite
**Severity: n/a · No action · the shape a coverage number cannot distinguish**
```
app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:550, :553
app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:349, :352, :278
```
F4 and F6 hold arms that cannot be *reached*. These can — and a test written against them would still
pass under the mutation that ought to redden it, which is the harder case to spot and the more
expensive one to discover halfway through writing the test.
- **`observer?.cancel()`'s non-null arm** (`ConversionViewModel:550`, `JoinViewModel:349`). Reachable
by calling `convert()` twice. But `ScreenOwnership`'s token is what actually blocks the superseded
write — the ViewModel's own KDoc at `reset()` says the cancel is "a request honoured at the next
suspension point" and "the claim is what actually stops that write". Delete `observer?.cancel()`
and the suite stays green, correctly.
- **`if (info == null) return@collect`** (`ConversionViewModel:553`, `JoinViewModel:352`). Reachable
through `pruneWork()`. But when the null arrives the state is already terminal, so removing the
guard crashes the collector and **leaves the state unchanged** — a state assertion is green under
the mutation. The only observable is an escaped coroutine exception, which the ViewModel's own KDoc
documents as unreliable on the JVM: kotlinx-coroutines-test's process-wide collector hands it to
whichever `runTest` starts next.
- **`JoinViewModel:278`'s `Ambiguous` arm.** Looks like the twin of `ReattachGuardsTest`'s "a result
two jobs both claim", and is not. An `Ambiguous` requires a shared `outputPath`, so it can only be a
*finished* job — which maps to `Joined`, a state that reads nothing from `inputs`. **The Convert-side
twin does bite**, because `displayNameOf(tags)` reaches the file card; the asymmetry is the point.
**Recorded because each of these was picked up as a candidate and put down again.** The wave-4 read
lost time to all three before the mutation test was run in the head rather than the editor, which is
the cheaper order.
---
## Summary
| ID | Finding | Severity | Evidence | Action |
@@ -449,23 +271,12 @@ the cheaper order.
| F3 | `ConversionRequest.videoCodec` / `.audioCodec` have no callers | low | confirmed by inspection | delete, or keep for symmetry — **not** a test gap |
| F4 | Two private guards reachable only by direct call | n/a | confirmed by inspection | **no action** — named exemption, per #88 |
| F5 | `ConversionNotifications.areEnabled()` is never called | low | confirmed by inspection; grep returns the declaration only | **decide**: act on it or delete it — **not** a test gap |
| F6 | Four more unreachable arms; `ConversionRouter:214-217`'s KDoc is false | low | confirmed by inspection; each traced to its upstream guard | **no action**, except the one-line KDoc fix |
| F7 | `probeWithExtractor`'s catch is unreachable, as `probeForConcat`'s is | n/a | measured across four URI shapes (recorded in `CLAUDE.md`) | **no action** — device-only, now written down for both sites |
| F8 | Three more dead members and six unused defaults | low | confirmed by inspection; grep per member | delete or keep knowingly — **not** a test gap |
| F9 | Both `getForegroundInfo` overrides are dead: expedited work is never used | n/a | confirmed by inspection; `grep setExpedited` returns nothing | **no action** — sharpens #88's close |
| F10 | Three reachable arms where no mutation bites | n/a | confirmed by inspection; each mutation traced to its masking guard | **no action** — recorded to stop the next read re-picking them |
Order, if these are acted on: **F1 and F5 first, separately.** They are the two with a possible
user-visible answer — a format the app can produce and does not offer, and a warning the app
documents and does not give — and either answer changes what the tidying should look like. F2, F3 and
F8 are tidying and belong in one commit with each other, not with F1 or F5. F6's KDoc correction is a
third kind: one line, no decision, and it should not wait on the tidying. F4, F7, F9 and F10 are
finished by being written down.
**Six of the ten are now "no action" or "not a test gap", and that is the useful shape.** By wave 4
the report's remaining red is mostly this: arms nothing can reach, members nothing calls, and arms a
test can reach but not pin. A coverage number cannot tell any of them from a real gap, which is why
this document exists and why it grows faster than the percentage moves.
documents and does not give — and either answer changes what the tidying should look like. F2 and F3
are tidying and belong in one commit with each other, not with F1 or F5. F4 is finished by being
written down.
**F1 and F5 share a shape worth naming:** both are places where a comment describes behaviour the
code does not have, and in both the tempting fix (delete the dead arm, test the dead method) would
@@ -476,15 +287,7 @@ freeze the wrong answer in place. The decision comes first.
**The test gaps from the same read.** Seven JVM-side gaps (**#132**) and three seam questions
(**#133**) came out of this coverage read and are tracked there, because they are work rather than
observations. This document holds only what a test would not fix. #133 also records why
`AndroidDeviceCodecs.probe()` was considered and left out **through `ShadowMediaCodecList`**, so that
spike is not run a third time.
**Updated 2026-09-02:** #194 proposes reaching the same code through a *pure seam* instead, which is a
different mechanism and one #133 did not evaluate — the builder objection it turns on (no
`setIsAlias`, no `setCanonicalName`) does not apply to a function taking its own entry type. #133's
close stands for the shadow; it is not a close on the seam. #194 also carries the reason the seam is
worth cutting at all, which is not coverage: the `runCatching` fallback logs "assuming permissive" and
returns empty sets, which makes `canEncode` and `canDecode` answer *no* for everything.
`AndroidDeviceCodecs.probe()` was considered and left out, so that spike is not run a third time.
**`ConversionForegroundType.current()`**, which looked like the sharpest gap in the read and is not.
Its API 33 and 34 arms are cold on the JVM, but issue **#88** already established that the class is
@@ -518,25 +321,6 @@ the real ones — **34 of 383** and **20 of 143** missed — and the screens are
better-covered files in the repo, which is what #52, #57 and #61 were for. **Do not chase the
branch number here.** If a future read wants a screen metric, use lines.
**Updated 2026-09-02: the same codegen inflates the *instruction* count, which wave 3's filter did
not allow for.** Wave 3 selected candidates on `mi > 0` — at least one missed instruction — which was
right to prefer over a bare branch count and is still wrong on these files. `JoinScreen.kt:222` reads
`mi=10` and looks uncovered; it also reads `ci=38`, and `JoinStateAffordancesTest` already clicks that
Save button and asserts `save:joined.mp4`. Every `onClick` lambda body flagged this way turned out to
be covered at method level, the missed instructions being the recomposition-skip path again.
Use `ci == 0` — the line never executed, which is JaCoCo's own missed-line definition — and pair it
with a method-level `ci > 0 && mb > 0` pass for covered methods with cold arms. Neither filter alone
is enough: `ConversionViewModel.cancel()` misses no line at all, yet its non-null arm had never been
entered in 584 tests (#192). `CLAUDE.md`'s coverage entry carries the same correction.
**Also codegen, also not gaps**, recorded once so they are not re-derived: the synthetic
`NoWhenBranchMatchedException` closing an exhaustive `when` (`ConverterScreen:399`, `:686`,
`JoinScreen:278`, `MainActivity:160`); the inner `is Idle -> Unit` arms at `ConverterScreen:253-254`
and `JoinScreen:158-159`, which are structurally unreachable because the outer `when` already routed
`Idle`; and the closing brace of a `launch` block whose `collect` never terminates
(`ConversionViewModel:578`, `JoinViewModel:371`).
**Anything requiring a device.** `MediaProbe`'s FFprobe half (`MediaProbe.kt:151, 156-158, 173-188`)
and `FFmpegEngine` in full report 0% on the JVM and are covered by `androidTest`. JaCoCo measures
`testDebugUnitTest` only; their zeroes are a boundary, as #84, #85, #86 and #88 each recorded