Merge branch 'main' into test/join-failure-message-on-device
This commit is contained in:
Jason Ross
@@ -310,6 +310,46 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
#218 and carries the unfixed scope. **Prefer a mutation that must go red to a repetition count**
|
||||
when a fix is for something intermittent.
|
||||
|
||||
**Every number above is `testDebugUnitTest` only, and on 2026-09-05 the instrumented suite got its
|
||||
first read for that reason** — `docs/e2e-read-findings.md`, entries **E1-E6**, tickets
|
||||
**#223-#230**. Four waves had been steered by a figure that **cannot see `app/src/androidTest` at
|
||||
all**, so nothing had ever asked what those 60 device tests pin, only that they were green.
|
||||
|
||||
**It found one test that passes while testing nothing, and it is the one that matters most.**
|
||||
`HardwareFallbackTest` is the only automated check of the hardware→software fallback against a
|
||||
*real* codec failure, and on run `34004304566` the API 33, 34, 35 and 37 legs each log
|
||||
`Routing sample_h264_444.mp4 -> ... via FFMPEG (NO_HARDWARE_ENCODER)` (API 36's logcat artifact on
|
||||
that run is truncated, so it is unread rather than different): emulators expose no
|
||||
hardware encoder, so the job never reaches Media3 and the `catch` it exists to prove is never
|
||||
entered. Its two assertions — succeeded, output non-empty — are true anyway, and it finishes in
|
||||
448 ms. **Deleting that `catch` reddens nothing on any leg** (#223).
|
||||
|
||||
Two things generalise from it. **A test can assert and still not reach**, which no coverage
|
||||
number and no "does it assert something" review would catch — the filter that works is *does this
|
||||
test's premise hold on the machine that runs it?*. And the codebase **already knew**: the sibling
|
||||
`ForcedFailureTest` pins `DeviceCodecs.PERMISSIVE` against exactly this hazard and writes out why,
|
||||
as does `ConversionWorkerTest`. The difference is that their assertions are about the *path*, so
|
||||
without the pin they would fail loudly; `HardwareFallbackTest`'s are about the *output*, so it
|
||||
passes quietly. **Prefer asserting the path over asserting the artefact** where the two differ.
|
||||
|
||||
The read was a triage, not a test push, and six of its seven findings are prose rather than code —
|
||||
the suite itself is in good shape. What had drifted is its self-description.
|
||||
|
||||
**Working the tickets then found the thing the read could not: one production defect.** #238 —
|
||||
joining files picked through the system picker failed outright on the stream-copy path. The
|
||||
concat demuxer whitelists protocols separately from `-safe 0`, and `ffkitsaf` was not on the
|
||||
list; only `STREAM_COPY` feeds it a list file, and every existing join test passed
|
||||
`Uri.fromFile`, so **the one broken combination was the only one a user could reach**. Not a
|
||||
missed line and not an unasserted value — two covered things no test put together, which is the
|
||||
gap shape a coverage number is worst at.
|
||||
|
||||
**E7 is the other reusable result**, because it re-scoped its own ticket. A real
|
||||
`DocumentsProvider` cannot be reached without the picker: an unprotected one is refused at
|
||||
install, instrumentation runs in the app's uid so the test APK's identity is no help, and shell
|
||||
identity is denied too — each denial naming `ACTION_OPEN_DOCUMENT`. So #226 has no cheap headless
|
||||
half. But the *input* bridge needs no documents provider at all, which is what kept #225 headless
|
||||
and is how #238 surfaced.
|
||||
|
||||
- **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.
|
||||
|
||||
@@ -42,6 +42,28 @@
|
||||
<action android:name="android.content.action.DOCUMENTS_PROVIDER" />
|
||||
</intent-filter>
|
||||
</provider>
|
||||
|
||||
<!--
|
||||
A PLAIN provider, for the ffkitsaf bridge on the success path.
|
||||
|
||||
FFmpegKitConfig.getSafParameterForRead is on every real user conversion and was on no
|
||||
passing test: they all pass Uri.fromFile, which takes the other arm. Only its failure
|
||||
side was covered, by UnopenableUriTest naming an authority that does not exist.
|
||||
|
||||
The documents provider above cannot serve this. Any DOCUMENTS_PROVIDER must hold
|
||||
MANAGE_DOCUMENTS or the platform refuses to install it, instrumentation runs in the
|
||||
target app's process and so carries the app's uid, and the resulting denial says what
|
||||
is actually required: access obtained through ACTION_OPEN_DOCUMENT. That means a picker,
|
||||
and the flake it brings. See issue #226.
|
||||
|
||||
The bridge does not need a documents provider. It opens a descriptor through the
|
||||
resolver and hands FFmpeg a saf: path, so any readable content:// URI exercises it, and
|
||||
an ordinary provider is allowed to be exported without a permission.
|
||||
-->
|
||||
<provider
|
||||
android:name="org.libremediaconverter.saf.FixtureContentProvider"
|
||||
android:authorities="org.libremediaconverter.test.content"
|
||||
android:exported="true" />
|
||||
</application>
|
||||
|
||||
</manifest>
|
||||
|
||||
@@ -17,7 +17,7 @@ package org.libremediaconverter
|
||||
*
|
||||
* Removing it is the goal, and the trigger is written down: a new API 37.x system image, or an
|
||||
* ATD image for 37. Delete the annotation from the tests, and the advisory job goes empty and
|
||||
* the gating one grows by two.
|
||||
* the gating one grows by [FAILS_ON_EMULATOR_API37_BASELINE].
|
||||
*
|
||||
* **How many tests carry it is committed below**, as [FAILS_ON_EMULATOR_API37_BASELINE], and the
|
||||
* advisory job checks the run against it. Adding or removing a marker means changing that number
|
||||
@@ -52,4 +52,4 @@ annotation class FailsOnEmulatorApi37
|
||||
* `INSTRUMENTATION_ABORTED`, so the count is a number taken from a partial run. The report
|
||||
* records the truncation next to the counts for that reason.
|
||||
*/
|
||||
const val FAILS_ON_EMULATOR_API37_BASELINE = 3
|
||||
const val FAILS_ON_EMULATOR_API37_BASELINE = 4
|
||||
|
||||
@@ -7,6 +7,10 @@ import androidx.media3.common.MimeTypes
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.cancelAndJoin
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import org.junit.After
|
||||
@@ -14,6 +18,7 @@ import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
@@ -262,6 +267,92 @@ class Media3EngineTest {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancelling a *running* export stops it, completing #224's third engine.
|
||||
*
|
||||
* The two FFmpeg engines were done first (`ad2a75d`, `d293646`); this is
|
||||
* `Media3Engine.transcode`'s `invokeOnCancellation`, which posts `transformer.cancel()` onto the
|
||||
* engine's own `HandlerThread` because `cancel()` has the same single-thread requirement as
|
||||
* `start()`.
|
||||
*
|
||||
* ## Why the assertion is the output file here, and was not for FFmpeg
|
||||
*
|
||||
* The FFmpeg side could not use the file: `invokeOnCancellation` unlinks it, and on POSIX ffmpeg
|
||||
* keeps writing to the unlinked inode, so the path stays gone whether or not the cancel landed.
|
||||
* It asserted the session's return code instead.
|
||||
*
|
||||
* `Media3Engine` deletes nothing — the partial is `ConversionWorker`'s to clean up — so the file
|
||||
* *is* the evidence. An export that was cancelled leaves no moov atom, so `MediaExtractor`
|
||||
* either finds no video track or refuses the file outright with
|
||||
* `IOException: Failed to instantiate extractor` — measured, and both mean interrupted. One
|
||||
* that ran to completion leaves a playable HEVC file, which is the only outcome treated as a
|
||||
* miss. The wait before
|
||||
* reading it is deliberately several times the length of the export, so a *non*-cancelled export
|
||||
* has certainly finished by then: the failure direction is "the file became valid", never "we
|
||||
* did not wait long enough".
|
||||
*
|
||||
* ## Why it retries
|
||||
*
|
||||
* Same reason as the other two, measured there: the committed fixture is 3 s at 320x240 and the
|
||||
* export outruns a naive cancel on a loaded runner. An attempt whose export finished before the
|
||||
* cancel landed has tested nothing, so it is a miss and is retried; only exhausting
|
||||
* [CANCEL_ATTEMPTS] fails. With `transformer.cancel()` removed every attempt produces a playable
|
||||
* file, so the mutation still bites — it just takes five tries to say so.
|
||||
*
|
||||
* Progress having been reported is what proves the export really started, so a miss is
|
||||
* distinguishable from an export that never ran at all — which matters on the API 37 image,
|
||||
* where the decoder is what fails.
|
||||
*/
|
||||
@Test
|
||||
@FailsOnEmulatorApi37
|
||||
fun cancellingARunningExportStopsIt(): Unit = runBlocking {
|
||||
val outcomes = mutableListOf<String>()
|
||||
|
||||
repeat(CANCEL_ATTEMPTS) { attempt ->
|
||||
val partial = File(context.cacheDir, "cancelled_export_$attempt.mp4").apply { delete() }
|
||||
|
||||
val job = launch(Dispatchers.IO) {
|
||||
engine.transcode(
|
||||
input = Uri.fromFile(input),
|
||||
output = partial,
|
||||
request = ConversionRequest(OutputFormat.MP4_H265.spec),
|
||||
)
|
||||
}
|
||||
|
||||
// The muxer creating the file is proof the export really started, and it is the
|
||||
// earliest such proof available -- earlier than the first progress tick.
|
||||
withTimeout(TIMEOUT_MS) {
|
||||
while (!partial.exists() && job.isActive) delay(POLL_MS)
|
||||
}
|
||||
val started = partial.exists()
|
||||
job.cancelAndJoin()
|
||||
|
||||
if (!started) {
|
||||
// The export failed before writing anything. That is not a cancellation result
|
||||
// either way, so it is not allowed to pass as one.
|
||||
outcomes += "attempt $attempt never produced an output file to cancel"
|
||||
return@repeat
|
||||
}
|
||||
|
||||
// Several times the export's own length, so a cancel that did not land has certainly
|
||||
// finished. The failure direction is "the file became playable", never "too soon".
|
||||
delay(SETTLE_MS)
|
||||
|
||||
// A cancelled export reports itself two ways and both mean the same thing: no video
|
||||
// track, or MediaExtractor refusing the file outright with "Failed to instantiate
|
||||
// extractor" because there is no moov atom to read. Only a *playable* file is a miss.
|
||||
val video = runCatching { videoMimeTypeOf(partial) }.getOrNull()
|
||||
partial.delete()
|
||||
if (video == null) return@runBlocking
|
||||
outcomes += "attempt $attempt produced a playable $video"
|
||||
}
|
||||
|
||||
fail(
|
||||
"never interrupted a running export in $CANCEL_ATTEMPTS attempts, so either every " +
|
||||
"export finished first or cancellation does not reach the transformer: $outcomes",
|
||||
)
|
||||
}
|
||||
|
||||
private fun videoMimeTypeOf(file: File): String? {
|
||||
val extractor = MediaExtractor()
|
||||
try {
|
||||
@@ -280,6 +371,19 @@ class Media3EngineTest {
|
||||
private companion object {
|
||||
const val TIMEOUT_SECONDS = 120L
|
||||
|
||||
/** Bounds the wait for the muxer to create the file; a hang here is a defect. */
|
||||
const val TIMEOUT_MS = 30_000L
|
||||
const val POLL_MS = 25L
|
||||
|
||||
/**
|
||||
* How long to let a *failed* cancel finish. Several times the export's own length, so
|
||||
* "the file is not playable" cannot mean "not yet".
|
||||
*/
|
||||
const val SETTLE_MS = 10_000L
|
||||
|
||||
/** See the KDoc: a miss is the loaded-runner case, not a defect. */
|
||||
const val CANCEL_ATTEMPTS = 5
|
||||
|
||||
/**
|
||||
* Short on purpose. Nothing is decoded or encoded on this path — the builder refuses the
|
||||
* input outright — so anything approaching this is a hang, which is what the test is
|
||||
|
||||
@@ -13,6 +13,7 @@ import androidx.work.WorkManager
|
||||
import androidx.work.Worker
|
||||
import androidx.work.WorkerParameters
|
||||
import androidx.work.workDataOf
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeout
|
||||
@@ -27,7 +28,10 @@ import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.join.JoinState
|
||||
import org.libremediaconverter.join.JoinViewModel
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import org.libremediaconverter.model.Engine
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import org.libremediaconverter.work.ConversionWorker
|
||||
import org.libremediaconverter.work.JobTags
|
||||
@@ -64,6 +68,26 @@ class EchoWorker(context: Context, params: WorkerParameters) : Worker(context, p
|
||||
* path, foreground service included — into a synchronous test double, depending on class order.
|
||||
*/
|
||||
@UnstableApi
|
||||
/**
|
||||
* A [SoftwareTranscoder] that holds the worker in [WorkInfo.State.RUNNING] until released.
|
||||
*
|
||||
* Declared here rather than in `FakeFailures` because it is the only test that needs a job to stay
|
||||
* live on demand, and the shape is specific to that: the others fake a *failure*, this fakes
|
||||
* *duration*.
|
||||
*/
|
||||
private class BlockingTranscoder(private val released: CompletableDeferred<Unit>) : SoftwareTranscoder {
|
||||
override suspend fun run(
|
||||
request: ConversionRequest,
|
||||
inputPath: String,
|
||||
output: File,
|
||||
durationMs: Long,
|
||||
onProgress: (Int) -> Unit,
|
||||
) {
|
||||
released.await()
|
||||
output.writeBytes(ByteArray(1_024))
|
||||
}
|
||||
}
|
||||
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class ReattachOnLaunchTest {
|
||||
|
||||
@@ -75,7 +99,13 @@ class ReattachOnLaunchTest {
|
||||
fun clearTheQueue() = emptyQueueAndStaging()
|
||||
|
||||
@After
|
||||
fun leaveNothingBehind() = emptyQueueAndStaging()
|
||||
fun leaveNothingBehind() {
|
||||
// The suite runs without Android Test Orchestrator, so every class shares one process and
|
||||
// a swapped seam outlives the class that set it. Only one test here swaps one, but a
|
||||
// BlockingTranscoder left in place would hang the next class that converts anything.
|
||||
ConversionDependencies.reset()
|
||||
emptyQueueAndStaging()
|
||||
}
|
||||
|
||||
/**
|
||||
* The claim the whole fix rests on, checked against the production request builder rather
|
||||
@@ -261,6 +291,69 @@ class ReattachOnLaunchTest {
|
||||
return request.id
|
||||
}
|
||||
|
||||
/**
|
||||
* Reattaching to a conversion that is **running right now**, which nothing had ever driven.
|
||||
*
|
||||
* This class covers a job that finished, one whose staged file is gone, an ambiguous pair, one
|
||||
* still queued, and one the user cancelled. [Reattachment.rank] gives
|
||||
* [WorkInfo.State.RUNNING] the **highest** rank of all — "live work outranks a finished result
|
||||
* because a running job is holding a foreground service" — and no test on either source set
|
||||
* ever produced one. `ReattachmentTest` exercises the ranking as a pure function over
|
||||
* fabricated snapshots; what was missing is a ViewModel meeting a real running job.
|
||||
*
|
||||
* It is also the likeliest reattachment there is: the user starts a conversion, leaves, and
|
||||
* comes back while it is still going.
|
||||
*
|
||||
* ## Why the engine is a fake here, and why that is not a weakening
|
||||
*
|
||||
* The job has to still be running when the ViewModel is built, and every real conversion in
|
||||
* this suite finishes in about a second — racing that is what made the cancellation tests flaky
|
||||
* enough to need retries (#224). A [SoftwareTranscoder] that blocks until released removes the
|
||||
* race outright: the job is `RUNNING` for exactly as long as the test wants.
|
||||
*
|
||||
* Nothing about reattachment depends on which engine is transcoding. What is under test is the
|
||||
* tag query, [Reattachment.choose] over live WorkManager state, and `observe` mapping it to
|
||||
* [ConversionState.Converting] — all of which run identically whatever is doing the work.
|
||||
*
|
||||
* ## What this does not do, and cannot (#230)
|
||||
*
|
||||
* It does not kill the process. `docs/defect-audit.md` D3/D13 record that `am kill` refuses a
|
||||
* process holding a foreground service, and there is a more basic obstacle: **instrumentation
|
||||
* runs in the app's own process**, so any route that really killed it would take the test
|
||||
* runner with it and there would be nothing left to assert with. A relaunch-and-observe test
|
||||
* needs two instrumentation runs, which the runner does not provide.
|
||||
*
|
||||
* So process death stays device-manual, and this is the closest observable analogue: a fresh
|
||||
* ViewModel, with no memory of the work, meeting a job that is genuinely mid-flight.
|
||||
*/
|
||||
@Test
|
||||
fun reattachesToAConversionThatIsStillRunning(): Unit = runBlocking {
|
||||
val released = CompletableDeferred<Unit>()
|
||||
ConversionDependencies.software = { BlockingTranscoder(released) }
|
||||
|
||||
val request = ConversionWorker.request(
|
||||
inputUri = Uri.fromFile(stage("running_input.mp3")),
|
||||
displayName = RUNNING_NAME,
|
||||
sizeBytes = RUNNING_SIZE,
|
||||
spec = OutputFormat.MP3.spec,
|
||||
quality = QualityTier.FAST,
|
||||
)
|
||||
workManager.enqueue(request).result.get()
|
||||
|
||||
// Deterministic: the worker cannot finish until this test lets it.
|
||||
withTimeout(TIMEOUT_MS) {
|
||||
workManager.getWorkInfoByIdFlow(request.id).first { it?.state == WorkInfo.State.RUNNING }
|
||||
}
|
||||
|
||||
val reattached = awaitConversion<ConversionState.Converting>()
|
||||
|
||||
assertEquals(RUNNING_NAME, reattached.input.displayName)
|
||||
assertEquals(RUNNING_SIZE, reattached.input.sizeBytes)
|
||||
|
||||
released.complete(Unit)
|
||||
workManager.cancelWorkById(request.id).result.get()
|
||||
}
|
||||
|
||||
/**
|
||||
* Enqueues a job that stays [WorkInfo.State.ENQUEUED]. The delay is what holds it there: it
|
||||
* is long enough that nothing can run it during a test, and it is cancelled either way.
|
||||
@@ -326,5 +419,9 @@ class ReattachOnLaunchTest {
|
||||
* against WorkManager's database, so this is generous rather than tuned.
|
||||
*/
|
||||
const val SETTLE_MS = 5_000L
|
||||
|
||||
/** Read back off the job's tags by the reattaching ViewModel, so both have to survive. */
|
||||
const val RUNNING_NAME = "still_running.mp3"
|
||||
const val RUNNING_SIZE = 4_242L
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,11 +12,17 @@ import kotlinx.coroutines.withTimeout
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assume.assumeTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.codec.AndroidDeviceCodecs
|
||||
import org.libremediaconverter.model.ConversionRequest
|
||||
import org.libremediaconverter.model.ConversionRouter
|
||||
import org.libremediaconverter.model.Engine
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import org.libremediaconverter.model.VideoCodec
|
||||
import org.libremediaconverter.work.ConversionWorker
|
||||
import java.io.File
|
||||
|
||||
@@ -34,6 +40,47 @@ import java.io.File
|
||||
* hand — a regression test that silently skips is worse than no test, because the count
|
||||
* still reads as coverage.
|
||||
*
|
||||
* ## Why this skips on emulators, and why that is the honest answer (#223)
|
||||
*
|
||||
* **This test used to pass everywhere while proving nothing.** Two independent facts stop the
|
||||
* fallback happening on an emulator, and both were measured rather than reasoned:
|
||||
*
|
||||
* 1. **The router never sends the job to Media3.** A Fast MP4/H.265 job goes to the hardware path
|
||||
* only when `device.canEncode(H265)`, and emulators expose no hardware encoder — every leg of
|
||||
* run `34004304566` logged
|
||||
* `Routing sample_h264_444.mp4 -> ... via FFMPEG (NO_HARDWARE_ENCODER)`. The whole test
|
||||
* finished in 448 ms, which is not long enough to fail an export and then re-encode.
|
||||
* 2. **Forcing it to Media3 does not help either, which is the part that settles it.** Pinning
|
||||
* `ConversionDependencies.deviceCodecs` to [DeviceCodecs.PERMISSIVE] — the trick
|
||||
* [ForcedFailureTest] uses — makes the router choose Media3, and the export then *succeeds*.
|
||||
* Measured on a local API 34 emulator: `MediaCodecInfo` logs
|
||||
* `NoSupport [codec.profileLevel, avc1.F4000C, video/avc]` for **both**
|
||||
* `c2.goldfish.h264.decoder` and `c2.android.avc.decoder`, and ExoPlayer allocates the
|
||||
* goldfish decoder anyway, which decodes the file regardless of the profile it declares.
|
||||
* `c2.android.hevc.encoder` then encodes the result and the job reports `MEDIA3`.
|
||||
*
|
||||
* So the class KDoc above — "Media3 fails partway through the export on every device" — **is not
|
||||
* true of the emulator images**, and no amount of routing pressure makes this fixture force a
|
||||
* fallback there. The emulator cannot answer this question, so the test says so out loud instead
|
||||
* of passing.
|
||||
*
|
||||
* That is why the gate is [assumeTrue] on the *production* premise (`canEncode(H265)`) rather than
|
||||
* a pinned profile: pinning would also swap in software codecs, which is not the path a real
|
||||
* device takes and is what made the forced run succeed. **This is now the third permanent skip**;
|
||||
* the other two are [org.libremediaconverter.bench.RealMediaBenchmark]'s.
|
||||
*
|
||||
* `ForcedFailureTest.hardwareFailureFallsBackToSoftware` still covers the fallback *wiring* on
|
||||
* every leg, with an `ExplodingHardware` double. What only a device with a real hardware encoder
|
||||
* can show is two real engines disagreeing about a real file, and that is what this is for.
|
||||
*
|
||||
* ## Why the assertion is a pair
|
||||
*
|
||||
* `KEY_ENGINE_USED` is `FFMPEG` whether the fallback fired **or** the router went straight there,
|
||||
* so asserting it alone would not have caught any of the above. The premise is asserted
|
||||
* separately: [ConversionRouter.route] chooses `MEDIA3` for this request on this device. Static
|
||||
* routing wanted hardware, the runtime result was software — together, and only together, that is
|
||||
* the fallback.
|
||||
*
|
||||
* The fixture was produced with x264, which the host toolchain cannot do (Fedora's
|
||||
* ffmpeg ships openh264, which is Constrained Baseline only):
|
||||
*
|
||||
@@ -66,6 +113,15 @@ class HardwareFallbackTest {
|
||||
|
||||
@Test
|
||||
fun aFileMedia3CannotDecodeStillConvertsViaFfmpeg(): Unit = runBlocking {
|
||||
// See "Why this skips on emulators" on the class. Without a real hardware encoder the
|
||||
// router never chooses Media3, and forcing it makes the export succeed instead of fail --
|
||||
// so there is no fallback to observe and a green run would mean nothing.
|
||||
assumeTrue(
|
||||
"no hardware HEVC encoder, so the router cannot choose Media3 and there is no " +
|
||||
"fallback to exercise",
|
||||
AndroidDeviceCodecs.get().canEncode(VideoCodec.H265),
|
||||
)
|
||||
|
||||
val request = ConversionWorker.request(
|
||||
inputUri = Uri.fromFile(input),
|
||||
displayName = SAMPLE,
|
||||
@@ -75,6 +131,19 @@ class HardwareFallbackTest {
|
||||
// the tier where the fallback has to rescue the conversion.
|
||||
quality = QualityTier.FAST,
|
||||
)
|
||||
// The premise, asserted rather than assumed: this request is one the router wants to send
|
||||
// to hardware on this device. Without it the test is green whether the fallback fired or
|
||||
// the job never went near Media3, which is exactly how #223 stayed invisible.
|
||||
val decision = ConversionRouter.route(
|
||||
ConversionRequest(OutputFormat.MP4_H265.spec, quality = QualityTier.FAST),
|
||||
AndroidDeviceCodecs.get(),
|
||||
)
|
||||
assertEquals(
|
||||
"this test only means something if the router sends this job to Media3",
|
||||
Engine.MEDIA3,
|
||||
decision.engine,
|
||||
)
|
||||
|
||||
workManager.enqueue(request).result.get()
|
||||
|
||||
val terminal = withTimeout(TIMEOUT_MS) {
|
||||
@@ -88,6 +157,14 @@ class HardwareFallbackTest {
|
||||
terminal?.state,
|
||||
)
|
||||
|
||||
// The outcome. Paired with the routing assertion above this is the fallback and nothing
|
||||
// else: hardware was chosen, software is what ran.
|
||||
assertEquals(
|
||||
"the router chose Media3, so a successful job must have fallen back to FFmpeg",
|
||||
Engine.FFMPEG.name,
|
||||
terminal?.outputData?.getString(ConversionWorker.KEY_ENGINE_USED),
|
||||
)
|
||||
|
||||
val out = File(terminal!!.outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)!!)
|
||||
assertTrue("no output produced", out.exists() && out.length() > 0)
|
||||
out.delete()
|
||||
|
||||
@@ -4,10 +4,20 @@ import android.media.MediaExtractor
|
||||
import android.media.MediaFormat
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
import com.arthenica.ffmpegkit.FFmpegKit
|
||||
import com.arthenica.ffmpegkit.FFmpegSession
|
||||
import com.arthenica.ffmpegkit.ReturnCode
|
||||
import com.arthenica.ffmpegkit.SessionState
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.cancelAndJoin
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
@@ -113,6 +123,11 @@ class FFmpegEngineTest {
|
||||
fun encodesFlacLosslessAudio() {
|
||||
val out = convert(OutputFormat.FLAC)
|
||||
assertTrue("no FLAC produced", out.exists() && out.length() > 0)
|
||||
// "fLaC", the native FLAC stream marker. Without this the test passed on any non-empty
|
||||
// file, so a builder arm emitting the wrong encoder into a .flac name shipped green
|
||||
// (#228) -- the same shape the five assertions above already guard against.
|
||||
val magic = out.inputStream().use { String(it.readNBytes(4), Charsets.US_ASCII) }
|
||||
assertEquals("fLaC", magic)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -127,6 +142,165 @@ class FFmpegEngineTest {
|
||||
fun encodesOpus() {
|
||||
val out = convert(OutputFormat.OPUS)
|
||||
assertTrue("no Opus produced", out.exists() && out.length() > 0)
|
||||
// OutputFormat.OPUS is Container.OGG, so the file is an Ogg stream: "OggS" (#228).
|
||||
// Deliberately the container marker rather than the codec -- it is what the other
|
||||
// container-level assertions in this class check, and it is four bytes at offset 0.
|
||||
val magic = out.inputStream().use { String(it.readNBytes(4), Charsets.US_ASCII) }
|
||||
assertEquals("OggS", magic)
|
||||
}
|
||||
|
||||
/**
|
||||
* The percentage itself, which every other test in this class computes and none of them reads.
|
||||
*
|
||||
* `FFmpegEngine` derives progress as `stats.time / durationMs * 100`, and the statistics
|
||||
* callback runs on every conversion here — but every call site omits `onProgress`, so until
|
||||
* this test nothing on any source set had ever looked at the number (#229). #196 covered the
|
||||
* *worker's* progress lambda, and did it with a fake engine that reports whatever the test
|
||||
* tells it to; `ProgressNotificationTest` covers throttling the same way. The arithmetic was
|
||||
* the one part with no reader.
|
||||
*
|
||||
* ## Why the duration is deliberately wrong
|
||||
*
|
||||
* `sample_h264.mp4` is exactly 3.000 s, and this passes **30 s** as the duration. So the
|
||||
* conversion still encodes the whole clip, `stats.time` still climbs to about 3000 ms, and the
|
||||
* reported percentage tops out around **10** rather than 100.
|
||||
*
|
||||
* That is what makes the assertion bite. A range check alone is worthless here: replacing
|
||||
* `percent` with a constant `0` satisfies "every value is in 0..100" and "the values never go
|
||||
* backwards", and so does a list of `[0, 100]`. Pinning the *band* rejects every constant, and
|
||||
* — because the band is a tenth of the way up — it also rejects an implementation that ignores
|
||||
* `durationMs`, which would report ~100 for the same run.
|
||||
*
|
||||
* The bound is deliberately loose (5..25 for an expected 10). The last statistics callback can
|
||||
* land slightly before the final frame, so the peak is "about 3000 ms of a claimed 30 000",
|
||||
* not exactly it.
|
||||
*/
|
||||
@Test
|
||||
fun progressIsReportedAsAFractionOfTheDurationItWasGiven() {
|
||||
val seen = mutableListOf<Int>()
|
||||
val out = outputFor("out_progress.mp4")
|
||||
runBlocking {
|
||||
engine.run(
|
||||
request = ConversionRequest(spec = OutputFormat.MP4_H264.spec, quality = QualityTier.BEST),
|
||||
inputPath = input.absolutePath,
|
||||
output = out,
|
||||
// Ten times the fixture's real 3 s. See the KDoc.
|
||||
durationMs = 30_000,
|
||||
onProgress = { percent -> seen += percent },
|
||||
)
|
||||
}
|
||||
|
||||
assertTrue("the statistics callback never reported progress", seen.isNotEmpty())
|
||||
assertTrue("progress out of range: $seen", seen.all { it in 0..100 })
|
||||
assertEquals("progress went backwards: $seen", seen.sorted(), seen)
|
||||
// The band. Rejects any constant, and rejects ignoring durationMs (which would read ~100).
|
||||
val peak = seen.max()
|
||||
assertTrue(
|
||||
"3 s of media against a claimed 30 s should peak near 10%, got $peak from $seen",
|
||||
peak in 5..25,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancelling a *running* conversion actually stops the native session.
|
||||
*
|
||||
* Nothing on any source set did this before (#224). Every `cancel` in `app/src/androidTest` is
|
||||
* `WorkManager.cancelWorkById` against work that is **queued or already finished** — the two in
|
||||
* `ReattachOnLaunchTest` cancel a job carrying a one-hour initial delay, and one immediately
|
||||
* after enqueue. On the JVM, `WorkerCancellationTest` and `HardwareFallbackTest`'s cancellation
|
||||
* case drive a `SoftwareTranscoder` double that records the call. No test had ever asked a real
|
||||
* native session to stop. This is `docs/defect-audit.md` **D10**'s forcing condition.
|
||||
*
|
||||
* It is the one path where cancelling wrong is silently expensive rather than loudly broken: a
|
||||
* missed `FFmpegKit.cancel` leaves the native process encoding to completion while the UI says
|
||||
* the job is cancelled, and nothing reports the battery and thermal cost.
|
||||
*
|
||||
* ## Why the assertion is the session's return code, not the output file
|
||||
*
|
||||
* The obvious assertion — the partial output is gone — **cannot fail**, so it would have been a
|
||||
* vacuous test. `invokeOnCancellation` deletes the path, and on POSIX unlinking a file ffmpeg
|
||||
* still holds open leaves ffmpeg writing to the unlinked inode; the path stays gone whether or
|
||||
* not the cancel ever reached the session. Deleting `FFmpegKit.cancel` and keeping
|
||||
* `output.delete()` passes that check every time.
|
||||
*
|
||||
* What distinguishes them is the session's own verdict: a cancelled session ends with the
|
||||
* cancel return code, a completed one ends successfully. That is a fact about the session
|
||||
* rather than about timing, so it is read *after* waiting for the session to leave
|
||||
* [SessionState.RUNNING] rather than at a fixed delay.
|
||||
*
|
||||
* ## Why it cancels on RUNNING rather than on the first progress callback
|
||||
*
|
||||
* Measured. Cancelling from the first `onProgress` was tried first and **failed on a local API
|
||||
* 34 emulator with `state=COMPLETED rc=0`** — every committed fixture is 2-3 s at 320x240, and
|
||||
* the encode finishes before the first statistics callback has been delivered and acted on. The
|
||||
* progress callback proves the session is running, but arrives too late to interrupt anything.
|
||||
* `FFmpegKit.listSessions` shows the session [SessionState.RUNNING] far earlier.
|
||||
*
|
||||
* ## Why it retries, which is the part that took two attempts to get right
|
||||
*
|
||||
* Waiting for `RUNNING` is not on its own enough. With `MP4_H265` at [QualityTier.BEST] this
|
||||
* passed four consecutive local runs and all five CI legs, then failed on the API 34 and 35 legs
|
||||
* of the next PR with `state=COMPLETED rc=0`. Nothing had changed: on a loaded runner the thread
|
||||
* that observed `RUNNING` can be descheduled long enough for a short encode to finish before it
|
||||
* calls `cancel`. A longer timeout does not help — the wait already succeeded.
|
||||
*
|
||||
* Two changes together, because neither is sufficient:
|
||||
*
|
||||
* - **A slower encode.** `WEBM_VP9` at `BEST` is the slowest thing this builder emits:
|
||||
* `libvpx-vp9 -crf 31 -b:v 0`, with `-deadline realtime` added **only** on
|
||||
* [QualityTier.FAST]. Probed on an API 34 emulator, that session is still `RUNNING` at 1 s
|
||||
* and finished by 2 s, against well under a second for x265 `-preset medium`.
|
||||
* - **Retrying the attempt.** An attempt whose session finished before the cancel landed has
|
||||
* not tested anything, so it is not a failure — it is a miss, and it is retried. Only
|
||||
* exhausting [CANCEL_ATTEMPTS] is a failure, and its message says which case it hit.
|
||||
*
|
||||
* That keeps the mutation honest: with `FFmpegKit.cancel` removed **every** attempt ends
|
||||
* `COMPLETED`, so the test still fails — it just takes [CANCEL_ATTEMPTS] tries to say so.
|
||||
*
|
||||
* The session is identified by diffing against the ids present before each attempt, because
|
||||
* this class has already produced eight of them by the time this executes.
|
||||
*/
|
||||
@Test
|
||||
fun cancellingARunningConversionCancelsTheNativeSession(): Unit = runBlocking {
|
||||
val outcomes = mutableListOf<String>()
|
||||
|
||||
repeat(CANCEL_ATTEMPTS) { attempt ->
|
||||
val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
|
||||
val out = outputFor("out_cancelled_$attempt.webm")
|
||||
|
||||
val job = launch(Dispatchers.IO) {
|
||||
engine.run(
|
||||
// The slowest target this builder emits -- see the KDoc. Not decoration:
|
||||
// with a faster one this loses the race on a loaded CI runner.
|
||||
request = ConversionRequest(spec = OutputFormat.WEBM_VP9.spec, quality = QualityTier.BEST),
|
||||
inputPath = input.absolutePath,
|
||||
output = out,
|
||||
durationMs = 3_000,
|
||||
)
|
||||
}
|
||||
|
||||
val ours = withTimeout(TIMEOUT_MS) {
|
||||
var found: FFmpegSession? = null
|
||||
while (found == null) {
|
||||
found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
|
||||
if (found == null) delay(POLL_MS)
|
||||
}
|
||||
found
|
||||
}
|
||||
job.cancelAndJoin()
|
||||
withTimeout(TIMEOUT_MS) {
|
||||
while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
|
||||
}
|
||||
|
||||
if (ReturnCode.isCancel(ours.getReturnCode())) return@runBlocking
|
||||
// The encode beat us to it. That attempt proved nothing either way, so try again.
|
||||
outcomes += "state=${ours.getState()} rc=${ours.getReturnCode()}"
|
||||
}
|
||||
|
||||
fail(
|
||||
"never interrupted a running session in $CANCEL_ATTEMPTS attempts, so either every " +
|
||||
"encode finished first or cancellation does not reach it: $outcomes",
|
||||
)
|
||||
}
|
||||
|
||||
// --- the quality tier the GPL licence was taken for --------------------
|
||||
@@ -166,4 +340,19 @@ class FFmpegEngineTest {
|
||||
}.exceptionOrNull()
|
||||
assertTrue("expected an FFmpegException, got $failure", failure is FFmpegEngine.FFmpegException)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** Generous: it bounds a hang, and every wait here normally settles in well under a second. */
|
||||
const val TIMEOUT_MS = 30_000L
|
||||
const val POLL_MS = 50L
|
||||
|
||||
/**
|
||||
* How many times to try to catch the session mid-encode.
|
||||
*
|
||||
* Each miss costs about the length of one VP9 encode -- a second or two -- and a miss is
|
||||
* the loaded-runner case rather than a defect. Five is enough that exhausting them means
|
||||
* cancellation is not reaching the session, which is what the failure message says.
|
||||
*/
|
||||
const val CANCEL_ATTEMPTS = 5
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,10 +5,20 @@ import android.media.MediaFormat
|
||||
import android.net.Uri
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
import com.arthenica.ffmpegkit.FFmpegKit
|
||||
import com.arthenica.ffmpegkit.FFmpegSession
|
||||
import com.arthenica.ffmpegkit.ReturnCode
|
||||
import com.arthenica.ffmpegkit.SessionState
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.cancelAndJoin
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
@@ -17,6 +27,7 @@ import org.libremediaconverter.convert.StagingNames
|
||||
import org.libremediaconverter.ffmpeg.ConcatEngine
|
||||
import org.libremediaconverter.ffmpeg.FFmpegEngine
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
@@ -51,6 +62,82 @@ class ConcatEngineTest {
|
||||
(staged + listOf(clipA, clipB, clipMismatched)).forEach { it.delete() }
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancelling a *running* join actually stops the native session.
|
||||
*
|
||||
* The `FFmpegEngine` half of #224 landed first (PR #236); this is the same gap in
|
||||
* [ConcatEngine]. Before these two, no test on any source set had ever asked a real native
|
||||
* session to stop — every `cancel` in `app/src/androidTest` targets WorkManager entries that
|
||||
* are queued or already finished.
|
||||
*
|
||||
* ## Two things carried over from the conversion side, both measured there
|
||||
*
|
||||
* **The assertion is the session's return code.** A cancelled session ends with the cancel
|
||||
* code, a completed one does not. The alternative — checking the output file — is even less
|
||||
* available here than it was for conversions: [ConcatEngine] does not delete its output on
|
||||
* cancellation at all. Its `invokeOnCancellation` is `FFmpegKit.cancel(...)` and nothing else,
|
||||
* where [org.libremediaconverter.ffmpeg.FFmpegEngine]'s also deletes the partial. Whether that
|
||||
* asymmetry is deliberate is a separate question from this test, which is why this asserts the
|
||||
* thing that is true of both.
|
||||
*
|
||||
* **The cancel is triggered on [SessionState.RUNNING], not on progress.** `ConcatWorker`
|
||||
* publishes no progress at all, so there is no callback to hang it on even in principle — but
|
||||
* the conversion side established the deeper reason: the committed clips are 2 s at 320x240 and
|
||||
* the encode outruns a callback-triggered cancel.
|
||||
*
|
||||
* **And the attempt is retried**, for the reason the conversion side measured the hard way: on
|
||||
* a loaded runner the thread that observed `RUNNING` can be descheduled long enough for a short
|
||||
* encode to finish before it calls `cancel`, which failed two CI legs there. An attempt whose
|
||||
* session finished first has tested nothing, so it is a miss rather than a failure; only
|
||||
* exhausting [CANCEL_ATTEMPTS] fails, and with `FFmpegKit.cancel` removed every attempt misses,
|
||||
* so the mutation still bites.
|
||||
*
|
||||
* The inputs are deliberately the **mismatched** pair, so [ConcatStrategy.REENCODE] is chosen.
|
||||
* A stream copy of two short clips is close to instantaneous and would leave nothing to
|
||||
* interrupt; re-encoding is the case where a user would actually reach for Cancel.
|
||||
*
|
||||
* *Mutation:* drop `FFmpegKit.cancel(session.getSessionId())` from `ConcatEngine`'s
|
||||
* `invokeOnCancellation` — the session runs to completion and this fails.
|
||||
*/
|
||||
@Test
|
||||
fun cancellingARunningJoinCancelsTheNativeSession(): Unit = runBlocking {
|
||||
val outcomes = mutableListOf<String>()
|
||||
|
||||
repeat(CANCEL_ATTEMPTS) { attempt ->
|
||||
val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
|
||||
val out = output("cancelled_join_$attempt.mp4")
|
||||
|
||||
val job = launch(Dispatchers.IO) {
|
||||
engine.join(
|
||||
listOf(Uri.fromFile(clipA), Uri.fromFile(clipMismatched)),
|
||||
out,
|
||||
ConcatWorker.DEFAULT_FORMAT,
|
||||
)
|
||||
}
|
||||
|
||||
val ours = withTimeout(TIMEOUT_MS) {
|
||||
var found: FFmpegSession? = null
|
||||
while (found == null) {
|
||||
found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
|
||||
if (found == null) delay(POLL_MS)
|
||||
}
|
||||
found
|
||||
}
|
||||
job.cancelAndJoin()
|
||||
withTimeout(TIMEOUT_MS) {
|
||||
while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
|
||||
}
|
||||
|
||||
if (ReturnCode.isCancel(ours.getReturnCode())) return@runBlocking
|
||||
outcomes += "state=${ours.getState()} rc=${ours.getReturnCode()}"
|
||||
}
|
||||
|
||||
fail(
|
||||
"never interrupted a running join in $CANCEL_ATTEMPTS attempts, so either every " +
|
||||
"encode finished first or cancellation does not reach it: $outcomes",
|
||||
)
|
||||
}
|
||||
|
||||
private fun copyAsset(name: String): File {
|
||||
val out = File(context.cacheDir, name)
|
||||
InstrumentationRegistry.getInstrumentation().context.assets
|
||||
@@ -230,4 +317,13 @@ class ConcatEngineTest {
|
||||
a.width != mismatched.width || a.height != mismatched.height,
|
||||
)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** Generous: it bounds a hang, and both waits here normally settle in well under a second. */
|
||||
const val TIMEOUT_MS = 30_000L
|
||||
const val POLL_MS = 50L
|
||||
|
||||
/** See the conversion side: a miss is the loaded-runner case, not a defect. */
|
||||
const val CANCEL_ATTEMPTS = 5
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
package org.libremediaconverter.saf
|
||||
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
import androidx.work.WorkInfo
|
||||
import androidx.work.WorkManager
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.ffmpeg.ConcatEngine
|
||||
import org.libremediaconverter.model.Engine
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import org.libremediaconverter.work.ConversionWorker
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* A `content://` input reaching FFmpeg successfully, which nothing had ever driven (#225).
|
||||
*
|
||||
* `FFmpegKitConfig.getSafParameterForRead` stands between a SAF grant and the native process, and
|
||||
* it is on **every real user conversion**. Every passing convert and join test in this suite hands
|
||||
* the worker a `Uri.fromFile(...)`, which takes the `uri.path` arm instead — so the bridge was
|
||||
* exercised only on its failure side, by `UnopenableUriTest` naming an authority that does not
|
||||
* exist. That proves the error message, not the bridge.
|
||||
*
|
||||
* ## Why a plain provider rather than the documents one
|
||||
*
|
||||
* [FixtureDocumentsProvider] cannot be reached from the app, measured three ways on an API 34
|
||||
* emulator (#226): a `DOCUMENTS_PROVIDER` declared without `MANAGE_DOCUMENTS` is refused at install
|
||||
* — *"Provider must be protected by MANAGE_DOCUMENTS"*; instrumentation runs in the **target app's
|
||||
* process**, so `Instrumentation.getContext()` still carries the app's uid and is denied; and
|
||||
* `adoptShellPermissionIdentity(MANAGE_DOCUMENTS)` is denied identically. The denial names the only
|
||||
* way in: *"you obtain access using ACTION_OPEN_DOCUMENT or related APIs"*.
|
||||
*
|
||||
* The bridge does not need one. It opens a descriptor through the resolver and hands FFmpeg a
|
||||
* `saf:` path, so any readable `content://` URI exercises it — and [FixtureContentProvider] is an
|
||||
* ordinary provider, which may be exported without a permission. The whole class is headless: no
|
||||
* DocumentsUI, and none of the flake #190 records.
|
||||
*
|
||||
* ## Why MP3
|
||||
*
|
||||
* The bridge lives on the FFmpeg arm, and MP3 is the format the router sends there unconditionally
|
||||
* — no platform encoder exists at any API level, so `ConversionWorkerTest.routesAnMp3JobToFfmpeg…`
|
||||
* relies on the same fact. Choosing a video target would make the engine depend on the device's
|
||||
* codecs, and #223 is what that costs.
|
||||
*
|
||||
* *Mutation:* make `getSafParameterForRead` return `uri.toString()`. FFmpeg cannot open it and both
|
||||
* tests fail; nothing else in either suite notices.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class ContentUriInputTest {
|
||||
|
||||
private val context = InstrumentationRegistry.getInstrumentation().targetContext
|
||||
private val workManager = WorkManager.getInstance(context)
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
File(context.cacheDir, "conversions").listFiles()?.forEach { it.delete() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aContentUriInputConvertsThroughTheSafBridge(): Unit = runBlocking {
|
||||
val input = FixtureContentProvider.uriFor(SAMPLE)
|
||||
val request = ConversionWorker.request(
|
||||
inputUri = input,
|
||||
displayName = SAMPLE,
|
||||
sizeBytes = 0L,
|
||||
spec = OutputFormat.MP3.spec,
|
||||
quality = QualityTier.FAST,
|
||||
)
|
||||
workManager.enqueue(request).result.get()
|
||||
|
||||
val terminal = withTimeout(TIMEOUT_MS) {
|
||||
workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
|
||||
}
|
||||
|
||||
val error = terminal?.outputData?.getString(ConversionWorker.KEY_ERROR)
|
||||
assertEquals(
|
||||
"a content:// input must convert, but failed with: $error",
|
||||
WorkInfo.State.SUCCEEDED,
|
||||
terminal?.state,
|
||||
)
|
||||
// The bridge is on the FFmpeg arm only, so this is part of the claim rather than colour.
|
||||
assertEquals(Engine.FFMPEG.name, terminal?.outputData?.getString(ConversionWorker.KEY_ENGINE_USED))
|
||||
|
||||
val out = File(terminal!!.outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)!!)
|
||||
assertTrue("no output produced from a content:// input", out.exists() && out.length() > 0)
|
||||
out.delete()
|
||||
}
|
||||
|
||||
/**
|
||||
* The same bridge on the join path, which has its own copy of the call (`ConcatEngine:36`).
|
||||
*
|
||||
* Driven through the engine rather than `ConcatWorker` because the engine is where the branch
|
||||
* is; the worker adds a foreground service and nothing else this is about.
|
||||
*/
|
||||
@Test
|
||||
fun contentUriInputsJoinThroughTheSafBridge(): Unit = runBlocking {
|
||||
val out = File(context.cacheDir, "joined_from_content.mp4").apply { delete() }
|
||||
val result = ConcatEngine(context).join(
|
||||
listOf(FixtureContentProvider.uriFor(CLIP_A), FixtureContentProvider.uriFor(CLIP_B)),
|
||||
out,
|
||||
OutputFormat.MP4_H264,
|
||||
)
|
||||
|
||||
assertTrue("no output produced from content:// inputs", result.output.length() > 0)
|
||||
out.delete()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val SAMPLE = "sample_h264.mp4"
|
||||
const val CLIP_A = "clip_a.mp4"
|
||||
const val CLIP_B = "clip_b.mp4"
|
||||
const val TIMEOUT_MS = 300_000L
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,135 @@
|
||||
package org.libremediaconverter.saf;
|
||||
|
||||
import android.content.ContentProvider;
|
||||
import android.content.ContentValues;
|
||||
import android.database.Cursor;
|
||||
import android.database.MatrixCursor;
|
||||
import android.net.Uri;
|
||||
import android.os.ParcelFileDescriptor;
|
||||
import android.provider.OpenableColumns;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileNotFoundException;
|
||||
import java.io.FileOutputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.io.OutputStream;
|
||||
|
||||
/**
|
||||
* A plain {@link ContentProvider} serving the committed media fixtures over {@code content://}.
|
||||
*
|
||||
* <p><b>Why this exists alongside {@link FixtureDocumentsProvider}.</b> Every passing convert and
|
||||
* join test hands the worker a {@code Uri.fromFile(...)}, which takes the {@code uri.path} arm and
|
||||
* never touches {@code FFmpegKitConfig.getSafParameterForRead}. That bridge is on 100% of real user
|
||||
* conversions and was on 0% of tested ones; only its failure side was covered, by
|
||||
* {@code UnopenableUriTest} pointing at an authority that does not exist.
|
||||
*
|
||||
* <p><b>Why not the documents provider.</b> It cannot be reached. Measured three ways on an API 34
|
||||
* emulator: a {@code DOCUMENTS_PROVIDER} declared without {@code MANAGE_DOCUMENTS} is refused at
|
||||
* install ("Provider must be protected by MANAGE_DOCUMENTS"); instrumentation runs in the target
|
||||
* app's process, so {@code Instrumentation.getContext()} still carries the app's uid and is denied;
|
||||
* and {@code adoptShellPermissionIdentity(MANAGE_DOCUMENTS)} is denied identically. The denial says
|
||||
* what is required — <i>"you obtain access using ACTION_OPEN_DOCUMENT or related APIs"</i> — so a
|
||||
* documents provider is reachable only through a picker-issued grant. See issue #226.
|
||||
*
|
||||
* <p>The bridge does not need one. {@code getSafParameterForRead} opens a file descriptor through
|
||||
* the resolver and hands FFmpeg a {@code saf:} path; any readable {@code content://} URI exercises
|
||||
* it. An ordinary provider may be exported without a permission, so this one is, and the whole test
|
||||
* stays headless — no DocumentsUI, and none of the flake #190 records.
|
||||
*
|
||||
* <p>Unlike {@link FixtureDocumentsProvider} this may use {@code androidx} and Kotlin freely — it is
|
||||
* loaded into the app process like any other provider, not into the bare test process. It is kept
|
||||
* in Java anyway, next to its sibling, so the two read alike.
|
||||
*/
|
||||
public final class FixtureContentProvider extends ContentProvider {
|
||||
|
||||
/** Authority. Distinct from the documents provider's, and from anything the app declares. */
|
||||
public static final String AUTHORITY = "org.libremediaconverter.test.content";
|
||||
|
||||
/** Builds a URI for one of this source set's committed assets, e.g. {@code sample_h264.mp4}. */
|
||||
public static Uri uriFor(String assetName) {
|
||||
return new Uri.Builder().scheme("content").authority(AUTHORITY).appendPath(assetName).build();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean onCreate() {
|
||||
return true;
|
||||
}
|
||||
|
||||
@Override
|
||||
public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException {
|
||||
if (!"r".equals(mode)) {
|
||||
throw new FileNotFoundException("this provider is read-only: " + mode);
|
||||
}
|
||||
return ParcelFileDescriptor.open(unpack(assetOf(uri)), ParcelFileDescriptor.MODE_READ_ONLY);
|
||||
}
|
||||
|
||||
/**
|
||||
* Enough of {@link OpenableColumns} for {@code InputQuery.describe} to name and size the input.
|
||||
*
|
||||
* <p>Without these the app reaches the "Size unknown" screen, which is a different test.
|
||||
*/
|
||||
@Override
|
||||
public Cursor query(Uri uri, String[] projection, String selection, String[] args, String sort) {
|
||||
String asset = assetOf(uri);
|
||||
File file;
|
||||
try {
|
||||
file = unpack(asset);
|
||||
} catch (FileNotFoundException e) {
|
||||
return null;
|
||||
}
|
||||
MatrixCursor cursor = new MatrixCursor(
|
||||
new String[] {OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE});
|
||||
cursor.newRow().add(OpenableColumns.DISPLAY_NAME, asset).add(OpenableColumns.SIZE, file.length());
|
||||
return cursor;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getType(Uri uri) {
|
||||
return assetOf(uri).endsWith(".m4a") ? "audio/mp4" : "video/mp4";
|
||||
}
|
||||
|
||||
@Override
|
||||
public Uri insert(Uri uri, ContentValues values) {
|
||||
throw new UnsupportedOperationException("read-only fixture provider");
|
||||
}
|
||||
|
||||
@Override
|
||||
public int delete(Uri uri, String selection, String[] args) {
|
||||
throw new UnsupportedOperationException("read-only fixture provider");
|
||||
}
|
||||
|
||||
@Override
|
||||
public int update(Uri uri, ContentValues values, String selection, String[] args) {
|
||||
throw new UnsupportedOperationException("read-only fixture provider");
|
||||
}
|
||||
|
||||
private static String assetOf(Uri uri) {
|
||||
String asset = uri.getLastPathSegment();
|
||||
return asset == null ? "" : asset;
|
||||
}
|
||||
|
||||
/**
|
||||
* The asset on disk, unpacked the first time anything asks.
|
||||
*
|
||||
* <p>Reported as {@link FileNotFoundException} rather than swallowed: a provider answering with
|
||||
* a zero-byte file would fail the conversion for a reason nothing states.
|
||||
*/
|
||||
private File unpack(String asset) throws FileNotFoundException {
|
||||
File file = new File(getContext().getCacheDir(), "provided_" + asset);
|
||||
if (file.length() > 0L) {
|
||||
return file;
|
||||
}
|
||||
try (InputStream source = getContext().getAssets().open(asset);
|
||||
OutputStream sink = new FileOutputStream(file)) {
|
||||
byte[] buffer = new byte[8192];
|
||||
int read;
|
||||
while ((read = source.read(buffer)) != -1) {
|
||||
sink.write(buffer, 0, read);
|
||||
}
|
||||
} catch (IOException e) {
|
||||
throw new FileNotFoundException("could not unpack " + asset + ": " + e);
|
||||
}
|
||||
return file;
|
||||
}
|
||||
}
|
||||
@@ -207,6 +207,8 @@ import java.util.concurrent.atomic.AtomicInteger
|
||||
* driven there at all. That is why this gap survived as long as it did.
|
||||
* `tools/local-emulator/run-e2e.sh` runs API 33-36 on the development host, and both tests pass
|
||||
* there: **59 / 0 / 0 / 2 at API 33 and again at API 36**, whole suite, 2026-08-24.
|
||||
* (Since #223 the skip column reads 3 on an emulator — `HardwareFallbackTest` now announces
|
||||
* that it cannot run without a hardware HEVC encoder rather than passing vacuously.)
|
||||
*
|
||||
* ### Why only the rotation test carries [FailsOnEmulatorApi37]
|
||||
*
|
||||
|
||||
+129
@@ -0,0 +1,129 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
import androidx.work.OneTimeWorkRequestBuilder
|
||||
import androidx.work.WorkInfo
|
||||
import androidx.work.WorkManager
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotNull
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.libremediaconverter.model.QualityTier
|
||||
import java.io.File
|
||||
import java.util.concurrent.TimeUnit
|
||||
|
||||
/**
|
||||
* The Cancel button in the notification shade actually cancels the job.
|
||||
*
|
||||
* `ConversionNotifications.build` attaches one action, wired to
|
||||
* `WorkManager.createCancelPendingIntent(id)`. Before this test `createCancelPendingIntent` had
|
||||
* **no references anywhere outside its own declaration** — no JVM test, no instrumented test
|
||||
* (#227).
|
||||
*
|
||||
* That matters more than an ordinary uncovered line. A conversion runs in a foreground service and
|
||||
* the user is invited to leave the app; once they do, this action is the only way to stop it. If
|
||||
* the `PendingIntent` carries the wrong id, the button does nothing, the notification stays, and
|
||||
* the job runs to completion — with no error, no log, and no screen to look at.
|
||||
*
|
||||
* ## Why this fires the intent rather than reading the shade
|
||||
*
|
||||
* The obvious version asks `NotificationManager.getActiveNotifications()` for id 1001 and taps what
|
||||
* it finds. That was rejected: the instrumented suite grants no runtime permissions, so
|
||||
* `POST_NOTIFICATIONS` is denied throughout, and whether a suppressed foreground-service
|
||||
* notification is returned there is a platform detail that varies — the test would be asserting
|
||||
* something about notification *visibility* rather than about cancellation.
|
||||
*
|
||||
* The `PendingIntent` is the subject; where it is read from is incidental. Building the
|
||||
* notification for a real, live work id and firing its action exercises exactly the thing that can
|
||||
* be wrong — a real `PendingIntent` dispatch reaching real `WorkManager` — and does it the same way
|
||||
* on every API level.
|
||||
*
|
||||
* ## Why the job is delayed rather than running
|
||||
*
|
||||
* A conversion of the committed 3 s fixture finishes in well under a second on an emulator
|
||||
* (`HardwareFallbackTest` completed one in 448 ms), so racing a cancel against a running job would
|
||||
* be flaky in the direction that fails. An initial delay keeps the job reliably `ENQUEUED`, which
|
||||
* is a state `cancelWorkById` acts on identically — what is under test is whether firing the action
|
||||
* reaches WorkManager with the right id, not which state it interrupts.
|
||||
*
|
||||
* *Mutation:* build the `PendingIntent` from `UUID.randomUUID()` instead of the request's id. The
|
||||
* notification looks identical and the job is never cancelled.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class NotificationCancelActionTest {
|
||||
|
||||
private val context = InstrumentationRegistry.getInstrumentation().targetContext
|
||||
private val workManager = WorkManager.getInstance(context)
|
||||
private lateinit var input: File
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
input = File(context.cacheDir, "cancel_action_sample.mp4")
|
||||
InstrumentationRegistry.getInstrumentation().context.assets
|
||||
.open("sample_h264.mp4")
|
||||
.use { asset -> input.outputStream().use { asset.copyTo(it) } }
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
input.delete()
|
||||
File(context.cacheDir, "conversions").listFiles()?.forEach { it.delete() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun theNotificationsCancelActionCancelsThatJob(): Unit = runBlocking {
|
||||
val request = ConversionWorker.request(
|
||||
inputUri = Uri.fromFile(input),
|
||||
displayName = input.name,
|
||||
sizeBytes = input.length(),
|
||||
spec = OutputFormat.MP4_H264.spec,
|
||||
quality = QualityTier.FAST,
|
||||
).let { base ->
|
||||
// Rebuild with a delay so the job stays ENQUEUED for the whole test. See the KDoc.
|
||||
OneTimeWorkRequestBuilder<ConversionWorker>()
|
||||
.setInputData(base.workSpec.input)
|
||||
.setInitialDelay(1, TimeUnit.HOURS)
|
||||
.build()
|
||||
}
|
||||
workManager.enqueue(request).result.get()
|
||||
|
||||
// The job is queued and waiting, which is the state the cancel has to interrupt.
|
||||
assertEquals(
|
||||
WorkInfo.State.ENQUEUED,
|
||||
withTimeout(TIMEOUT_MS) {
|
||||
workManager.getWorkInfoByIdFlow(request.id).first { it != null }
|
||||
}?.state,
|
||||
)
|
||||
|
||||
val notification = ConversionNotifications(context)
|
||||
.build(request.id, title = input.name, percent = 0, indeterminate = true)
|
||||
val action = notification.actions?.firstOrNull()
|
||||
assertNotNull("the progress notification carries no action to cancel with", action)
|
||||
|
||||
// The whole point: fire it the way the shade would, and see the job stop.
|
||||
action!!.actionIntent.send()
|
||||
|
||||
val terminal = withTimeout(TIMEOUT_MS) {
|
||||
workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
|
||||
}
|
||||
assertEquals(
|
||||
"firing the notification's Cancel action must cancel the job it was built for",
|
||||
WorkInfo.State.CANCELLED,
|
||||
terminal?.state,
|
||||
)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val TIMEOUT_MS = 30_000L
|
||||
}
|
||||
}
|
||||
@@ -35,6 +35,21 @@ object FFmpegConcatCommand {
|
||||
add("concat")
|
||||
add("-safe")
|
||||
add("0")
|
||||
// And -protocol_whitelist permits the *scheme* those paths carry, which is a
|
||||
// separate gate (#238). Every input the user actually picks is a content:// URI --
|
||||
// JoinScreen uses OpenMultipleDocuments -- so ConcatEngine maps it through
|
||||
// FFmpegKitConfig.getSafParameterForRead and writes an `ffkitsaf:` path into the
|
||||
// list file. The concat demuxer applies its own whitelist, defaulting to
|
||||
// "file,crypto,data", and refused every one of them:
|
||||
//
|
||||
// [ffkitsaf @ ...] Protocol 'ffkitsaf' not on whitelist 'file,crypto,data'!
|
||||
//
|
||||
// This only widens that default. It is on the stream-copy branch alone because it
|
||||
// is the only one that feeds the demuxer a list file -- REENCODE passes each input
|
||||
// with its own -i, where the whitelist does not apply, which is why joining over SAF
|
||||
// worked for mismatched clips and failed for matching ones.
|
||||
add("-protocol_whitelist")
|
||||
add(PROTOCOL_WHITELIST)
|
||||
add("-i")
|
||||
add(listFile.absolutePath)
|
||||
add("-c")
|
||||
@@ -84,4 +99,12 @@ object FFmpegConcatCommand {
|
||||
add(output.absolutePath)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The concat demuxer's protocol whitelist: FFmpeg's own default, plus ffmpeg-kit's SAF scheme.
|
||||
*
|
||||
* Spelled out rather than appended to an unknown default, because the default is FFmpeg's and
|
||||
* could change under us; naming all four keeps the command self-describing. See #238.
|
||||
*/
|
||||
private const val PROTOCOL_WHITELIST = "file,crypto,data,ffkitsaf"
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import android.net.Uri
|
||||
import androidx.activity.ComponentActivity
|
||||
import androidx.compose.ui.test.assertIsDisplayed
|
||||
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
||||
import androidx.compose.ui.test.onAllNodesWithTag
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
@@ -77,6 +78,28 @@ class LauncherWiringTest {
|
||||
* The transposition guard. A picked file has to reach `onInputPicked`, which is observable as
|
||||
* the screen arriving at `Ready` with the file card showing — `save()` from `Idle` returns at
|
||||
* its own guard and leaves nothing behind.
|
||||
*
|
||||
* ## Why this waits rather than asserting straight away (#220)
|
||||
*
|
||||
* `onInputPicked` does not reach `Ready` on the calling thread. It hops twice —
|
||||
* `withContext(pickDispatcher) { InputQuery.describe(...) }` and then the probe — and
|
||||
* `pickDispatcher` defaults to `Dispatchers.IO`, a real background thread that Compose's
|
||||
* idling does not know about. `deliver` therefore returns with the state still `Idle` more
|
||||
* often than not, and asserting immediately was a race the test usually won.
|
||||
*
|
||||
* It lost five times on CI in one day, on PRs whose diffs were instrumented tests and
|
||||
* documentation, which is what #220 was filed for. `waitUntil` polls through
|
||||
* `waitForIdle`, so it drains the main looper each time round and sees the recomposition that
|
||||
* the IO hop eventually posts back.
|
||||
*
|
||||
* **Injecting the dispatcher would be better and is not available here.** `pickDispatcher` is
|
||||
* a constructor parameter precisely so a test can pin it, but this test composes the real
|
||||
* `ConverterScreen`, which resolves its own ViewModel through `viewModel()` — the seam exists
|
||||
* one layer below the thing under test. Pinning it would mean not testing the launcher edge,
|
||||
* which is the whole point of this class.
|
||||
*
|
||||
* The wait does not weaken the assertion: transposing the two callbacks leaves the screen in
|
||||
* `Idle` forever, so it fails on the timeout with the same meaning it failed with before.
|
||||
*/
|
||||
@Test
|
||||
fun `a picked document is loaded as input rather than saved to`() {
|
||||
@@ -85,6 +108,11 @@ class LauncherWiringTest {
|
||||
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).performClick()
|
||||
deliver(Uri.parse("content://test/holiday.mkv"))
|
||||
|
||||
composeRule.waitUntil(PICK_TIMEOUT_MS) {
|
||||
composeRule.onAllNodesWithTag(TestTags.Converter.FILE_CARD_NAME)
|
||||
.fetchSemanticsNodes()
|
||||
.isNotEmpty()
|
||||
}
|
||||
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertIsDisplayed()
|
||||
}
|
||||
|
||||
@@ -138,4 +166,13 @@ class LauncherWiringTest {
|
||||
)
|
||||
composeRule.waitForIdle()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/**
|
||||
* Long enough that a slow CI runner is not the reason this fails, short enough that a
|
||||
* genuinely transposed callback does not stall the suite. The pick normally lands in
|
||||
* single-digit milliseconds.
|
||||
*/
|
||||
const val PICK_TIMEOUT_MS = 10_000L
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,6 +56,33 @@ class FFmpegConcatCommandTest {
|
||||
assertEquals("0", args[args.indexOf("-safe") + 1])
|
||||
}
|
||||
|
||||
/**
|
||||
* The gate that `-safe 0` does not open, and the one every real join needs (#238).
|
||||
*
|
||||
* `-safe 0` permits absolute *paths*; the concat demuxer separately whitelists the *protocol*,
|
||||
* defaulting to `file,crypto,data`. `JoinScreen` picks with `OpenMultipleDocuments`, so real
|
||||
* inputs are `content://` and `ConcatEngine` writes `ffkitsaf:` paths into the list file — which
|
||||
* the demuxer refused outright, failing every stream-copy join a user could actually start.
|
||||
*
|
||||
* The re-encode strategy has no equivalent assertion because it needs none: it passes each
|
||||
* input with its own `-i` and never feeds the demuxer a list file. That asymmetry is exactly
|
||||
* why the defect survived — joining mismatched clips over SAF worked.
|
||||
*/
|
||||
@Test
|
||||
fun `stream copy whitelists the protocol its list file entries actually use`() {
|
||||
val args = FFmpegConcatCommand.build(
|
||||
ConcatStrategy.STREAM_COPY,
|
||||
inputs,
|
||||
listFile,
|
||||
output,
|
||||
OutputFormat.MP4_H264,
|
||||
)
|
||||
val whitelist = args[args.indexOf("-protocol_whitelist") + 1].split(",")
|
||||
assertTrue("ffmpeg-kit's SAF scheme must be permitted, got $whitelist", "ffkitsaf" in whitelist)
|
||||
// The defaults have to survive too: the list file itself is opened over `file`.
|
||||
assertTrue("the demuxer still reads the list file itself, got $whitelist", "file" in whitelist)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `re-encode passes every input separately and builds a filter graph`() {
|
||||
val args = FFmpegConcatCommand.build(
|
||||
|
||||
@@ -0,0 +1,382 @@
|
||||
# E2E-read findings
|
||||
|
||||
**Status:** seven findings; E4 fixed, the rest standing, none urgent — **plus one confirmed vacuous test, which is a
|
||||
ticket rather than an entry here** (see [Not covered here](#not-covered-here)). `E1`–`E6` came from
|
||||
the 2026-09-05 read of the instrumented suite. Every entry here is a *test-suite* observation —
|
||||
something a new test would not fix, because the test already exists and the problem is what it
|
||||
claims rather than what it runs.
|
||||
**Scope:** what reading all 60 instrumented tests turned up that writing a 61st would not fix.
|
||||
**Last verified:** `main` at `4b02294`, 2026-09-05. **60 `@Test` methods in 12 classes**, three
|
||||
carrying `@FailsOnEmulatorApi37`, gating API 37 leg 57.
|
||||
|
||||
## Why this document exists, and why it is separate from the other two
|
||||
|
||||
`docs/coverage-read-findings.md` (`F1`–`F10`) came from reading a **JaCoCo report**, and JaCoCo
|
||||
measures `testDebugUnitTest` only. So four waves of coverage work have been shaped by a number that
|
||||
**cannot see `app/src/androidTest` at all**. The instrumented suite has never had the equivalent
|
||||
read: nothing has asked what those 60 tests actually pin, only that they are green.
|
||||
|
||||
That is the gap this read is in. It is a **triage, not a test push** — the same shape as wave 4's
|
||||
read, which "moved no number at all, and that is its result".
|
||||
|
||||
`docs/defect-audit.md` (`D1`–`D16`) is the record of things *wrong at runtime*. Nothing here is
|
||||
wrong at runtime. These are tests whose names, KDoc or reputation overstate what they execute.
|
||||
|
||||
Entry ids are `E1`–`E6` so they cannot be confused with `F1`–`F10` or `D1`–`D16`.
|
||||
|
||||
## How to read the confidence labels
|
||||
|
||||
Same vocabulary as the other two documents, deliberately:
|
||||
|
||||
- **Confirmed by inspection** — the control flow is fully readable and the finding follows from it.
|
||||
- **Confirmed by measurement** — observed in a CI artifact, with the run id recorded.
|
||||
- **No action** — recorded because it looks like a finding and is not.
|
||||
|
||||
## The method, and the one filter that found everything
|
||||
|
||||
A coverage number is useless here by construction, so the read used a different question, applied
|
||||
to every one of the 60 tests:
|
||||
|
||||
> **If the behaviour this test is named for stopped working, would it go red?**
|
||||
|
||||
Three answers, and only the third is a gap:
|
||||
|
||||
- **yes** — the test bites. Most of the suite.
|
||||
- **no, and that is deliberate and written down** — `RealMediaBenchmark` asserts nothing on purpose
|
||||
(E2); `transcodesH264ToH265AndReportsProgress` declines to assert progress for a stated reason
|
||||
(E3). These are entries here, not tickets.
|
||||
- **no, and nothing says so** — the gap. One test, and it is the most important one in the suite.
|
||||
|
||||
**The reusable part is the second filter**, because "does it assert something?" would have cleared
|
||||
the vacuous test — it asserts two things. What it does not do is *reach the code it names*:
|
||||
|
||||
> **Does the test's own premise hold on the machine that runs it?**
|
||||
|
||||
`HardwareFallbackTest` asserts `SUCCEEDED` and a non-empty output, and both are true of a
|
||||
conversion that never went near the path it exists to prove (**#223**). See
|
||||
[Not covered here](#not-covered-here); it is filed rather than recorded here because a test fixes it.
|
||||
|
||||
---
|
||||
|
||||
## E1 — `RemuxTest`'s class KDoc argues for engine assertions three of its tests do not make, and they are right not to
|
||||
|
||||
**Severity: low · Confirmed by inspection · the KDoc is what is wrong, not the tests**
|
||||
|
||||
```
|
||||
app/src/androidTest/java/org/libremediaconverter/convert/RemuxTest.kt:31-42
|
||||
```
|
||||
|
||||
The class KDoc is headed **"Why these assert the engine, not just the file"** and makes a specific
|
||||
argument:
|
||||
|
||||
> A remux routed to FFmpeg produces a perfectly correct file — `-c copy` moves the same samples
|
||||
> into the same container. So an output-only assertion passes whether the hardware transmux path
|
||||
> ran or never executed at all […] which makes "silently always FFmpeg" the most likely way for
|
||||
> this feature to regress.
|
||||
|
||||
Five of its seven tests run a conversion. **Three assert no engine at all:**
|
||||
|
||||
| test | output container | asserts engine? |
|
||||
|---|---|---|
|
||||
| `mkvToMp4RemuxesOnHardware` | MP4 | **yes** — `MEDIA3` |
|
||||
| `mp4ToMkvRemuxesOnFFmpeg` | MKV | **yes** — `FFMPEG` |
|
||||
| `webmToMkvKeepsVp9WithoutReencoding` | MKV | no |
|
||||
| `audioOnlySourceRemuxesIntoMka` | MKV (`.mka`) | no |
|
||||
| `mp4ToMpegTsAndAviProduceTheirOwnContainers` | MPEG-TS, then AVI | **TS only**; the AVI half does not |
|
||||
|
||||
### Why this is not a gap
|
||||
|
||||
`ConversionRouter.MEDIA3_CONTAINERS = setOf(Container.MP4)` (`ConversionRouter.kt:37`), and every
|
||||
one of the three produces MKV or AVI. **They can only ever be FFmpeg**, so the regression the KDoc
|
||||
names — "silently always FFmpeg" — is not a thing that can happen to them. The two tests where the
|
||||
hardware path is genuinely at risk are exactly the two that assert it.
|
||||
|
||||
An engine assertion on the other three would be near-tautological given today's router. It would
|
||||
catch one thing: somebody adding MKV or AVI to `MEDIA3_CONTAINERS` without a muxer to match — which
|
||||
is what `Media3MuxersTest` is for, on the JVM, where it does not need a device.
|
||||
|
||||
### Why it is recorded rather than dropped
|
||||
|
||||
**This was the strongest-looking candidate of the whole read and it dissolved on tracing**, which
|
||||
is the same shape as `F5` in the coverage document (filed as a test gap, and only stopped being one
|
||||
when someone went looking for its callers). Recorded so the next read does not re-file it.
|
||||
|
||||
**The fix is one line of KDoc**, not three tests: the class asserts the engine *where the engine is
|
||||
in doubt*, which is a better rule than the one it currently states.
|
||||
|
||||
---
|
||||
|
||||
## E2 — three of the 60 instrumented tests assert nothing, and two of them never run
|
||||
|
||||
**Severity: n/a · No action — deliberate, documented, and load-bearing as documentation**
|
||||
|
||||
```
|
||||
app/src/androidTest/java/org/libremediaconverter/bench/RealMediaBenchmark.kt:25-53
|
||||
```
|
||||
|
||||
`reportDeviceEncoderCapabilities` logs and asserts nothing. `hardwareVersusSoftwareOnRealVideo` and
|
||||
`av1InputRoutesAccordingToDeviceDecodeSupport` are `assumeTrue`-guarded on media that is **not
|
||||
committed** and must be staged by hand into the app's internal `filesDir`, so they skip in every
|
||||
automated run — they are the "2 skipped" every green leg reports, and `docs/local-emulator.md:305`
|
||||
says so.
|
||||
|
||||
The class KDoc is unambiguous: *"This is a benchmark, not part of the automated suite […] Not a
|
||||
correctness test — the assertions are deliberately loose."*
|
||||
|
||||
**No action.** Recorded for one reason: **the suite's headline number is 60, and three of those 60
|
||||
are not tests.** Any future statement of the form "60 instrumented tests cover X" is off by three,
|
||||
and two of the three have never executed on CI at all.
|
||||
|
||||
**It is the opposite of E-nothing, though** — `reportDeviceEncoderCapabilities` runs on every leg
|
||||
and logs `BENCH can-encode:`, and **that log line is what confirmed the vacuous test this read
|
||||
found** (**#223**). An assertion-free test that prints the machine's capabilities turned out to be
|
||||
the only oracle in the suite. See [Not covered here](#not-covered-here).
|
||||
|
||||
---
|
||||
|
||||
## E3 — `transcodesH264ToH265AndReportsProgress` does not assert that progress was reported
|
||||
|
||||
**Severity: low · No action on the test; the name is the inaccurate part**
|
||||
|
||||
```
|
||||
app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt:73, :90-93
|
||||
```
|
||||
|
||||
```kotlin
|
||||
// Deliberately NOT asserting that progress fired. Polling is on a 250 ms tick,
|
||||
// and a 3 s 320x240 clip can finish inside one tick on fast hardware, which
|
||||
// would make the assertion fail intermittently for no real defect.
|
||||
seen.forEach { assertTrue("progress out of range: $it", it in 0..100) }
|
||||
```
|
||||
|
||||
`seen` is empty-safe: `forEach` on an empty list asserts nothing, so replacing `onProgress` with a
|
||||
no-op reddens nothing here. The reasoning is sound and the alternative really is a flaky test.
|
||||
|
||||
**No action on the body.** The name says `AndReportsProgress` and the body says it does not check
|
||||
that, which is the `probeForConcat` shape from `CLAUDE.md` — *a passing test with a wrong
|
||||
explanation is its own failure mode* — in its mildest form, since here the KDoc immediately corrects
|
||||
the name.
|
||||
|
||||
**Contrast the FFmpeg side, which is a real gap and is filed as #229**: `FFmpegEngine`'s percentage
|
||||
arithmetic is executed by every FFmpeg test and observed by none, because every call site omits
|
||||
`onProgress` entirely. Media3's is unasserted; FFmpeg's is unobserved. Only the second is a ticket.
|
||||
|
||||
---
|
||||
|
||||
## E4 — the marker's KDoc says removing it grows the gating leg by two; three tests carry it
|
||||
|
||||
**Severity: low · Confirmed by inspection · one line**
|
||||
|
||||
```
|
||||
app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt:20
|
||||
```
|
||||
|
||||
> Delete the annotation from the tests, and the advisory job goes empty and the gating one grows by
|
||||
> **two**.
|
||||
|
||||
Three tests carry it — `Media3EngineTest:72`, `Media3EngineTest:135`, `SafPickerRoundTripTest:320` —
|
||||
and `FAILS_ON_EMULATOR_API37_BASELINE = 3` eleven lines further down the same file, where the count
|
||||
is machine-checked by `.github/scripts/e2e-report-shape.sh`.
|
||||
|
||||
The third marker was added when the SAF rotation test was excluded; the sentence was not updated
|
||||
with it. **Everything that is checked is consistent at three**; only the prose says two, which is
|
||||
exactly why it drifted — and a good argument for the baseline const being a const.
|
||||
|
||||
---
|
||||
|
||||
## E5 — `coverage-read-findings.md`'s F7 calls covered code uncovered
|
||||
|
||||
**Severity: low · Confirmed by inspection · half of F7 is stale**
|
||||
|
||||
F7 says `probeWithExtractor`'s catch (`MediaProbe.kt:180-182`) is unreachable on Robolectric and
|
||||
"stays device-only", measured across four URI shapes. **The unreachability claim is correct and
|
||||
stands.** The implication readers take from it — that nothing exercises it — does not:
|
||||
|
||||
```
|
||||
app/src/androidTest/java/org/libremediaconverter/convert/RemuxTest.kt:111
|
||||
```
|
||||
|
||||
`probeDistinguishesAudioFromImagesFromRubbish` feeds it a file of random bytes and asserts
|
||||
`InputKind.UNPARSEABLE`, on a device, on every gating leg.
|
||||
|
||||
**"Device-only" holds; "uncovered" does not** — and the difference matters, because F7 is one of the
|
||||
six entries that document calls "no action", on the grounds that a test would not help. A test
|
||||
already exists. The entry should say so.
|
||||
|
||||
**This is the failure mode the split between the two documents was meant to prevent**, and it caught
|
||||
this repo out: a JaCoCo-derived document cannot see `androidTest`, so it will keep re-deriving
|
||||
"uncovered" for anything the instrumented suite covers. That is a structural reason for this
|
||||
document to exist, not a one-off correction.
|
||||
|
||||
---
|
||||
|
||||
## E6 — the suite's one device-capability assertion derives its expectation from the call it is testing
|
||||
|
||||
**Severity: low · Confirmed by inspection · no independent oracle exists**
|
||||
|
||||
```
|
||||
app/src/androidTest/java/org/libremediaconverter/work/ConversionWorkerTest.kt:151-152
|
||||
```
|
||||
|
||||
```kotlin
|
||||
val hasHardwareHevc = AndroidDeviceCodecs.get().canEncode(VideoCodec.H265)
|
||||
```
|
||||
|
||||
and then the expectation is `if (hasHardwareHevc) MEDIA3 else FFMPEG`. The test asks
|
||||
`AndroidDeviceCodecs` what to expect and then checks that the router agreed with
|
||||
`AndroidDeviceCodecs`. **If the whole enumeration returned empty, this would still pass** — and
|
||||
empty is precisely what the `runCatching` fallback returns (the reason `#194` was worth cutting;
|
||||
it logs "assuming permissive" while making `canEncode` answer *no* for everything).
|
||||
|
||||
Its KDoc defends the choice, and the defence is good:
|
||||
|
||||
> Asserting MEDIA3 unconditionally tests the test machine, not the router.
|
||||
|
||||
That is true, and there is no third source of truth on a device: `MediaCodecList` is what
|
||||
`AndroidDeviceCodecs` reads, so any oracle built from it is the same oracle.
|
||||
|
||||
**No action, but read it with #223.** It is the same missing oracle that makes the
|
||||
vacuous-test fix a judgement call rather than a one-liner — you cannot assert "this device has
|
||||
hardware HEVC" from inside the suite without asking the class under test. The honest options are a
|
||||
visible skip or a red test, and that decision is the ticket's.
|
||||
|
||||
---
|
||||
|
||||
## E7 — a real `DocumentsProvider` cannot be reached without the picker, so there is no cheap SAF test
|
||||
|
||||
**Severity: n/a · Confirmed by measurement · this is a platform rule, not a gap**
|
||||
|
||||
Added 2026-09-06, from doing #225 and #226 rather than from reading.
|
||||
|
||||
`OutputPublisher.publish`'s destination side is asserted only against Robolectric fakes —
|
||||
`FakeSafProvider`, registered with `asDocumentsProvider = true`, which is the flag that *makes*
|
||||
`DocumentsContract.isDocumentUri` answer true. #226 split that into a cheap headless half (drive a
|
||||
real `DocumentsProvider` directly) and an expensive picker-driven half.
|
||||
|
||||
**The cheap half does not exist.** Three approaches, all measured on an API 34 emulator:
|
||||
|
||||
| approach | result |
|
||||
|---|---|
|
||||
| a second `DOCUMENTS_PROVIDER` declared **without** `MANAGE_DOCUMENTS` | refused at install: `SecurityException: Provider must be protected by MANAGE_DOCUMENTS` |
|
||||
| create the document as the **test APK**, which owns the provider | denied — instrumentation runs *in the target app's process*, so it carries the app's uid whatever `Context` is asked |
|
||||
| `uiAutomation.adoptShellPermissionIdentity(MANAGE_DOCUMENTS)` | denied identically |
|
||||
|
||||
The denial names the only way in:
|
||||
|
||||
> `Permission Denial: opening provider …FixtureDocumentsProvider from
|
||||
> ProcessRecord{… org.libremediaconverter/u0a192} requires that you obtain access using
|
||||
> ACTION_OPEN_DOCUMENT or related APIs`
|
||||
|
||||
And the intent filter is not optional: without it `isDocumentUri` returns false, which is exactly
|
||||
the branch guarding `deletePartialOutput` — so a provider without the filter tests nothing the
|
||||
ticket is about.
|
||||
|
||||
**So any test of `publish` against a real `DocumentsProvider` must drive DocumentsUI**, and pays
|
||||
#190's flake tax. The work is one item at that cost, not two, and #226 was updated to say so.
|
||||
|
||||
### What this does *not* block, which is the useful half
|
||||
|
||||
`FFmpegKitConfig.getSafParameterForRead` — the bridge on every real conversion and join — needs no
|
||||
documents provider. It opens a descriptor through the resolver, so **any readable `content://` URI
|
||||
exercises it**, and an ordinary `ContentProvider` may be exported without a permission. That is what
|
||||
`FixtureContentProvider` is, and it made #225 headless.
|
||||
|
||||
**That distinction was worth the trouble**: the first test ever to hand the join path a real
|
||||
`content://` input found #238, a defect that broke joining for every user who picks matched files.
|
||||
The expensive gate protects the *destination* side; the *input* side never needed it.
|
||||
|
||||
## Summary
|
||||
|
||||
| ID | Finding | Severity | Evidence | Action |
|
||||
|---|---|---|---|---|
|
||||
| E1 | `RemuxTest`'s KDoc claims engine assertions three of its tests correctly omit | low | confirmed by inspection; traced through `MEDIA3_CONTAINERS` | **one line of KDoc** — the tests are right |
|
||||
| E2 | Three of the 60 instrumented tests assert nothing; two never run | n/a | confirmed by inspection; `docs/local-emulator.md:305` | **no action** — deliberate; but 60 ≠ 60 |
|
||||
| E3 | `…AndReportsProgress` does not assert progress fired | low | confirmed by inspection; reason inline | **no action** — the name overstates, the KDoc corrects it |
|
||||
| E4 | The API 37 marker's KDoc says "two"; three tests carry it | low | confirmed by inspection; baseline const says 3 | **fixed** in #243 — it names the constant now |
|
||||
| E5 | `coverage-read-findings.md` F7's "uncovered" half is stale | low | confirmed by inspection; `RemuxTest.kt:111` drives it | **amend F7** — "device-only" stands, "uncovered" does not |
|
||||
| E6 | The device-capability assertion asks the class under test what to expect | low | confirmed by inspection; no third oracle exists on a device | **no action** — read with **#223** |
|
||||
| E7 | A real `DocumentsProvider` is unreachable without the picker, so #226 has no cheap half | n/a | measured three ways on API 34; each denial names `ACTION_OPEN_DOCUMENT` | **no action** — it re-scoped #226 |
|
||||
|
||||
**Six of the seven are prose, not code**, and that is the shape of this read. The instrumented suite
|
||||
is in good condition: 57 of its 60 tests bite, the fixtures are committed with their generation
|
||||
recipes, and the one class that asserts nothing says so in its first line. What this read found is
|
||||
that **the suite's self-description has drifted from the suite** in five small places and one large
|
||||
one.
|
||||
|
||||
**The large one is not in this table**, because a test fixes it: **#223**.
|
||||
|
||||
## Not covered here
|
||||
|
||||
**The vacuous test.** `HardwareFallbackTest.aFileMedia3CannotDecodeStillConvertsViaFfmpeg` passes on
|
||||
every CI leg without ever entering the fallback it exists to prove. It is **#223**, not an entry
|
||||
here, because a test fixes it — and it is the reason this read happened rather than an aside from it.
|
||||
|
||||
Measured, not inferred, on run **`34004304566`** (all legs green), from each leg's own
|
||||
`e2e-diagnostics-api*` logcat:
|
||||
|
||||
```
|
||||
I/AndroidDeviceCodecs: Hardware video encoders: []
|
||||
I/RealMediaBenchmark: BENCH can-encode: COPY=true, H264=false, H265=false, VP9=false, VP8=false, AV1=false
|
||||
I/ConversionWorker: Routing sample_h264_444.mp4 -> OutputSpec(container=MP4, videoCodec=H265,
|
||||
audioCodec=AAC) via FFMPEG (NO_HARDWARE_ENCODER)
|
||||
```
|
||||
|
||||
Identical on **API 33, 34, 35 and 37**. (API 36's logcat artifact on that run is truncated to 838 KB
|
||||
and carries no test output at all, so it is unread rather than different.) The job is routed
|
||||
**straight to FFmpeg before Media3 is attempted**, the `catch` in `runMedia3OrFallBack` is never
|
||||
entered, and the test's two assertions — `SUCCEEDED`, output non-empty — are true anyway. It ran in
|
||||
448 ms.
|
||||
|
||||
**The repository already knew.** `ForcedFailureTest.hardwareFailureFallsBackToSoftware`, in the same
|
||||
package, pins `ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }` and says why:
|
||||
|
||||
> most emulators expose no hardware video encoder at all -- so the router would legitimately send
|
||||
> the job straight to FFmpeg and the hardware path would never be attempted. Without this the test
|
||||
> passes on a Pixel and fails on every emulator, which says nothing about the code under test.
|
||||
|
||||
`ConversionWorkerTest.routesAFastMp4JobByDeviceCapability` records the same fact a third time. The
|
||||
knowledge is in two sibling files; `HardwareFallbackTest` is the one that walked into it — and
|
||||
because its assertions are about the *output* rather than the *path*, it passes where
|
||||
`ForcedFailureTest` would have failed. **That asymmetry is why nobody noticed.**
|
||||
|
||||
**State it precisely.** The fallback *wiring* is covered on every leg by `ForcedFailureTest`, with
|
||||
fakes. What has never run on any emulator is a fallback triggered by a **real** mid-export codec
|
||||
failure — which is the case `HardwareFallbackTest` exists for, and the only reason
|
||||
`sample_h264_444.mp4` is committed at all. That fixture, generated with x264 because Fedora's
|
||||
ffmpeg ships openh264 and cannot produce High 4:4:4, does nothing on any CI leg today.
|
||||
|
||||
The fix is not one assertion. `KEY_ENGINE_USED` is `FFMPEG` **whether the fallback fired or the
|
||||
router went straight there** — asserting it changes nothing. The vacuity guard is two facts
|
||||
together: the router chose `MEDIA3` for this request on this device, *and* the worker reported
|
||||
`FFMPEG`. Whether to reach that with `assumeTrue` (a visible skip on emulators, and the "2 skipped"
|
||||
becomes 3) or with an assertion (red on emulators, announcing it cannot test what it claims) is a
|
||||
decision, not a detail — see **E6** for why no third option exists — and **#223** leaves it open.
|
||||
|
||||
**The other e2e gaps this read found are tickets too**, and are not repeated here:
|
||||
|
||||
| # | Gap |
|
||||
|---|---|
|
||||
| # | Gap | Outcome |
|
||||
|---|---|---|
|
||||
| **#223** | `HardwareFallbackTest` never attempts the hardware path on any emulator leg | closed — it skips instead of passing vacuously |
|
||||
| **#224** | Cancelling a *running* native session, in any of the three engines | closed — all three engines |
|
||||
| **#225** | No `content://` input has reached a *successful* conversion — the ffkitsaf bridge | closed, and it found **#238** |
|
||||
| **#226** | `OutputPublisher.publish` against a real `DocumentsProvider` | **open** — re-scoped by E7; one picker-driven item, not two |
|
||||
| **#227** | The notification's Cancel action has never been fired | closed |
|
||||
| **#228** | `encodesFlacLosslessAudio` and `encodesOpus` pass on any non-empty file | closed |
|
||||
| **#229** | FFmpeg's progress percentage is computed everywhere and asserted nowhere | closed |
|
||||
| **#230** | *(spike)* whether a running conversion's process can be killed | closed — it cannot; the runner shares the app's process |
|
||||
|
||||
**The read's own result, once the tickets were worked: one production defect.** #238 — joining files
|
||||
picked through the system picker failed outright on the stream-copy path, because the concat demuxer
|
||||
whitelists protocols separately from `-safe 0` and `ffkitsaf` was not on the list. Only `STREAM_COPY`
|
||||
feeds the demuxer a list file, and every existing join test passed `Uri.fromFile`, so the one broken
|
||||
combination was the only one a user could reach.
|
||||
|
||||
That is the argument for this kind of read in one line: the gap was not a missed line or an
|
||||
unasserted value, it was **a combination of two covered things that no test put together**.
|
||||
|
||||
**Nothing here was filed as a coverage delta.** Each names the mutation that has to go red, which is
|
||||
the acceptance criterion wave 4 established and which caught two vacuous tests in that wave before
|
||||
they shipped. #223 is the one that shows why the criterion matters: it has two passing assertions and
|
||||
still tests nothing.
|
||||
@@ -312,6 +312,18 @@ on sample media that is deliberately not committed. Its third test,
|
||||
`reportDeviceEncoderCapabilities`, has no such guard and runs. A level reporting 0 skipped
|
||||
would mean someone had staged sample files, not that something improved.
|
||||
|
||||
**Since #223 there is a third, and it is the interesting one.**
|
||||
`HardwareFallbackTest.aFileMedia3CannotDecodeStillConvertsViaFfmpeg` is `assumeTrue`-guarded on
|
||||
`AndroidDeviceCodecs.get().canEncode(H265)`, which is false on every emulator image — so it now
|
||||
skips here and runs only on the Pixel. It used to *pass* on emulators without ever attempting the
|
||||
hardware path, which is worse. **Expect `skipped="3"` locally**, and note the guard is a property
|
||||
of the machine rather than of staged files: a level reporting 2 would mean an emulator image had
|
||||
gained a hardware HEVC encoder, which is worth knowing.
|
||||
|
||||
That test's KDoc carries the measurement, including the part that decides it: forcing the route to
|
||||
Media3 anyway does *not* produce a fallback, because the goldfish decoder decodes the High 4:4:4
|
||||
fixture despite declaring `NoSupport` for its profile.
|
||||
|
||||
### What the sweep adds, and what it does not
|
||||
|
||||
**The renderer rule held four more times.** No boot log contains the string
|
||||
|
||||
Reference in New Issue
Block a user