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
11 changed files with 37 additions and 785 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
@@ -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,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
}
}
@@ -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,30 +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")
}
@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
@@ -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")
}
}