Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
  JMR-devandClaude Opus 5
|
cae050fcfb |
+37
-18
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user