Compare commits
30
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e0412329ff | ||
|
|
54932e97c6 | ||
|
|
ba16f5a89b | ||
|
|
bffcff92c7 | ||
|
|
9f06eb9988 | ||
|
|
06ca167034 | ||
|
|
c757565d64 | ||
|
|
4b02294cfb | ||
|
|
79097a0256 | ||
|
|
6004398a83 | ||
|
|
a354620bf5 | ||
|
|
17c91081cd | ||
|
|
20f718842d | ||
|
|
dec7089b59 | ||
|
|
b677a9ad02 | ||
|
|
34e4ab52a4 | ||
|
|
1437157a8f | ||
|
|
1041faf920 | ||
|
|
fe68f839c1 | ||
|
|
f65578b1f7 | ||
|
|
b38ad6a683 | ||
|
|
9a0f494e26 | ||
|
|
f3478706b3 | ||
|
|
61c400d2c6 | ||
|
|
d83775d5c6 | ||
|
|
e90f5a801c | ||
|
|
68015b3374 | ||
|
|
32ab54da3c | ||
|
|
e7caeeac43 | ||
|
|
ec2cae256f |
@@ -130,9 +130,9 @@ install for code that can never run — and on API 37 the full APK does not fit
|
|||||||
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
|
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
|
||||||
decision layer, where one branch is one documented user-visible outcome and the metric counts
|
decision layer, where one branch is one documented user-visible outcome and the metric counts
|
||||||
answers rather than complexity. Every other rule still applies there.
|
answers rather than complexity. Every other rule still applies there.
|
||||||
- **Coverage is reported, not gated** — **92.8% of lines (2183/2352), 81.3% of branches
|
- **Coverage is reported, not gated** — **94.2% of lines (2234/2372), 87.5% of branches
|
||||||
(1091/1342)**, measured 2026-09-02 with `./gradlew :app:jacocoTestReport`, against 584 JVM tests
|
(1171/1338)**, measured 2026-09-05 with `./gradlew :app:jacocoTestReport`, against 628 JVM tests
|
||||||
in 87 classes.
|
in 96 classes.
|
||||||
|
|
||||||
**Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
|
**Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
|
||||||
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
|
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
|
||||||
@@ -283,6 +283,58 @@ install for code that can never run — and on API 37 the full APK does not fit
|
|||||||
#194 before re-arguing either way — and note the reason it is worth cutting is not coverage but
|
#194 before re-arguing either way — and note the reason it is worth cutting is not coverage but
|
||||||
that the `runCatching` fallback logs "assuming permissive" while returning empty sets, which makes
|
that the `runCatching` fallback logs "assuming permissive" while returning empty sets, which makes
|
||||||
`canEncode` and `canDecode` answer *no* for everything.
|
`canEncode` and `canDecode` answer *no* for everything.
|
||||||
|
|
||||||
|
**Wave 4's tests then landed on 2026-09-05**, as #206-#217 for the twelve tickets plus #218
|
||||||
|
(#159) and #219 (#122): 92.8% -> **94.2%** line, 81.3% -> **87.5%** branch, 584 -> 628 tests in 87
|
||||||
|
-> 96 classes. Missed lines 169 -> 138, missed branches 251 -> 167.
|
||||||
|
|
||||||
|
**Its branch move is a different animal from the 2026-08-29 seam work's, and the difference is the
|
||||||
|
point.** That one gained 6.3 branch points with the numerator up 37 (974 -> 1011) while the
|
||||||
|
denominator *fell* 70 (1410 -> 1340) — much of the rise was scaffolding leaving the measurement
|
||||||
|
rather than arms being covered. Here the numerator is up **80** (1091 -> 1171) and
|
||||||
|
the denominator moved **-4** (1342 -> 1338). So this one is almost entirely tests choosing arms
|
||||||
|
nothing had chosen, which is what the entry above warns to check before quoting a branch figure.
|
||||||
|
The line denominator rose the other way, 2352 -> 2372, and that is new production code rather than
|
||||||
|
untested code: the seams the wave cut — `capabilitiesFrom`, `ffprobeInfoFrom`, `sessionOutcome`,
|
||||||
|
and `sweepScope`/`startupSweep`.
|
||||||
|
|
||||||
|
**The two-filter method above is what found the work**, and its second filter earned its place:
|
||||||
|
the largest single gap of the wave (#192, the Cancel button never shown to reach WorkManager) sits
|
||||||
|
on lines that were already green and no line-level filter could see it.
|
||||||
|
|
||||||
|
One result worth carrying forward about *evidence* rather than coverage. #218 fixed a flake whose
|
||||||
|
reproduction is statistical, and running the whole suite six times per arm caught nothing either
|
||||||
|
way — at the observed rate a clean six-run arm is roughly a coin flip, so the comparison was
|
||||||
|
underpowered and proved nothing. What settled it was a deterministic mutation, and then the merge
|
||||||
|
train confirmed it by accident: the race reproduced on #217's Unit tests leg, which sits below
|
||||||
|
#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 five of its six findings are prose rather than code —
|
||||||
|
the suite itself is in good shape. What had drifted is its self-description.
|
||||||
|
|
||||||
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
|
- **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 —
|
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.
|
a change that is both needs both.
|
||||||
@@ -410,4 +462,29 @@ Because versions float, a build can change without a commit. `./gradlew :app:dep
|
|||||||
run instead is `timeout` on the `Test` tasks plus the jstack watchdog beside it in
|
run instead is `timeout` on the `Test` tasks plus the jstack watchdog beside it in
|
||||||
`app/build.gradle.kts`, neither of which moves a thread. `HangBoundTest` guards both numbers,
|
`app/build.gradle.kts`, neither of which moves a thread. `HangBoundTest` guards both numbers,
|
||||||
and **a timed-out run writes no XML for the class that hung** — the dump is its only
|
and **a timed-out run writes no XML for the class that hung** — the dump is its only
|
||||||
attribution, so do not delete the watchdog as stray config.
|
attribution, so do not delete the watchdog as stray config. It has since been exercised in anger:
|
||||||
|
on 2026-09-05 it caught #125's Room/WorkManager deadlock on CI, failing in 10m57s with the hung
|
||||||
|
test named, where that ticket had predicted a 60-minute cap and no cause. #125 is closed as
|
||||||
|
bounded on the strength of it — the inversion itself is internal to the two libraries and still
|
||||||
|
live at `work-runtime` 2.11.2 / `room` 2.7.0.
|
||||||
|
- **The JVM suite does not run `LibreMediaConverterApp`.** `app/src/test/resources/robolectric.properties`
|
||||||
|
names `TestLibreMediaConverterApp` for every test, and it differs from the real class in exactly
|
||||||
|
one thing: `sweepScope` is `Dispatchers.Unconfined`, so the startup staging sweep finishes before
|
||||||
|
`onCreate()` returns instead of running on `Dispatchers.IO`.
|
||||||
|
|
||||||
|
**That line is load-bearing — do not delete it as stray config.** Robolectric builds an
|
||||||
|
`Application` per test class that asks for one, and each `onCreate` launched a sweep over the
|
||||||
|
shared `<cacheDir>/conversions/` that nothing joined. So a test asserting about a staged file was
|
||||||
|
racing every sweep the classes before it had left in flight (#159). It was CI-only until wave 4
|
||||||
|
added ten Robolectric classes, at which point `OutputPublisherStagingTest` failed on roughly one
|
||||||
|
local run in six. Per-test opt-in was measured and rejected: **27 of the 58 Robolectric classes
|
||||||
|
touch that directory**. The `SupervisorJob` is kept in the test scope so a throwing sweep is
|
||||||
|
swallowed there exactly as in production — the dispatcher is the only intended difference.
|
||||||
|
|
||||||
|
**It cost one assertion, knowingly.** `AppStartSweepTest` used to open by asserting that the
|
||||||
|
manifest's `android:name` is what Robolectric instantiated, so the sweep is code that actually
|
||||||
|
runs. An `application=` override *replaces* the manifest rather than being checked against it, and
|
||||||
|
`applicationInfo.className` reports the override too — measured — so that claim is not merely
|
||||||
|
unasserted on the JVM now, it is unobservable, and a rewritten version would assert the override
|
||||||
|
against itself. **The manifest link is device-only.** What remains is the `as LibreMediaConverterApp`
|
||||||
|
cast in that class's `setUp`, which catches only the test app ceasing to extend the real one.
|
||||||
|
|||||||
@@ -12,11 +12,17 @@ import kotlinx.coroutines.withTimeout
|
|||||||
import org.junit.After
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
|
import org.junit.Assume.assumeTrue
|
||||||
import org.junit.Before
|
import org.junit.Before
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
import org.junit.runner.RunWith
|
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.OutputFormat
|
||||||
import org.libremediaconverter.model.QualityTier
|
import org.libremediaconverter.model.QualityTier
|
||||||
|
import org.libremediaconverter.model.VideoCodec
|
||||||
import org.libremediaconverter.work.ConversionWorker
|
import org.libremediaconverter.work.ConversionWorker
|
||||||
import java.io.File
|
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
|
* hand — a regression test that silently skips is worse than no test, because the count
|
||||||
* still reads as coverage.
|
* 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
|
* The fixture was produced with x264, which the host toolchain cannot do (Fedora's
|
||||||
* ffmpeg ships openh264, which is Constrained Baseline only):
|
* ffmpeg ships openh264, which is Constrained Baseline only):
|
||||||
*
|
*
|
||||||
@@ -66,6 +113,15 @@ class HardwareFallbackTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun aFileMedia3CannotDecodeStillConvertsViaFfmpeg(): Unit = runBlocking {
|
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(
|
val request = ConversionWorker.request(
|
||||||
inputUri = Uri.fromFile(input),
|
inputUri = Uri.fromFile(input),
|
||||||
displayName = SAMPLE,
|
displayName = SAMPLE,
|
||||||
@@ -75,6 +131,19 @@ class HardwareFallbackTest {
|
|||||||
// the tier where the fallback has to rescue the conversion.
|
// the tier where the fallback has to rescue the conversion.
|
||||||
quality = QualityTier.FAST,
|
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()
|
workManager.enqueue(request).result.get()
|
||||||
|
|
||||||
val terminal = withTimeout(TIMEOUT_MS) {
|
val terminal = withTimeout(TIMEOUT_MS) {
|
||||||
@@ -88,6 +157,14 @@ class HardwareFallbackTest {
|
|||||||
terminal?.state,
|
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)!!)
|
val out = File(terminal!!.outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)!!)
|
||||||
assertTrue("no output produced", out.exists() && out.length() > 0)
|
assertTrue("no output produced", out.exists() && out.length() > 0)
|
||||||
out.delete()
|
out.delete()
|
||||||
|
|||||||
@@ -113,6 +113,11 @@ class FFmpegEngineTest {
|
|||||||
fun encodesFlacLosslessAudio() {
|
fun encodesFlacLosslessAudio() {
|
||||||
val out = convert(OutputFormat.FLAC)
|
val out = convert(OutputFormat.FLAC)
|
||||||
assertTrue("no FLAC produced", out.exists() && out.length() > 0)
|
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
|
@Test
|
||||||
@@ -127,6 +132,11 @@ class FFmpegEngineTest {
|
|||||||
fun encodesOpus() {
|
fun encodesOpus() {
|
||||||
val out = convert(OutputFormat.OPUS)
|
val out = convert(OutputFormat.OPUS)
|
||||||
assertTrue("no Opus produced", out.exists() && out.length() > 0)
|
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 quality tier the GPL licence was taken for --------------------
|
// --- the quality tier the GPL licence was taken for --------------------
|
||||||
|
|||||||
@@ -10,6 +10,9 @@ import androidx.compose.ui.test.performClick
|
|||||||
import androidx.media3.common.util.UnstableApi
|
import androidx.media3.common.util.UnstableApi
|
||||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||||
import androidx.test.platform.app.InstrumentationRegistry
|
import androidx.test.platform.app.InstrumentationRegistry
|
||||||
|
import androidx.test.runner.lifecycle.ActivityLifecycleCallback
|
||||||
|
import androidx.test.runner.lifecycle.ActivityLifecycleMonitorRegistry
|
||||||
|
import androidx.test.runner.lifecycle.Stage
|
||||||
import androidx.test.uiautomator.By
|
import androidx.test.uiautomator.By
|
||||||
import androidx.test.uiautomator.BySelector
|
import androidx.test.uiautomator.BySelector
|
||||||
import androidx.test.uiautomator.Configurator
|
import androidx.test.uiautomator.Configurator
|
||||||
@@ -24,6 +27,7 @@ import org.junit.runner.RunWith
|
|||||||
import org.libremediaconverter.FailsOnEmulatorApi37
|
import org.libremediaconverter.FailsOnEmulatorApi37
|
||||||
import org.libremediaconverter.MainActivity
|
import org.libremediaconverter.MainActivity
|
||||||
import org.libremediaconverter.ui.TestTags
|
import org.libremediaconverter.ui.TestTags
|
||||||
|
import java.util.concurrent.atomic.AtomicInteger
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Choosing a file, through the real system picker, and still having it after a rotation.
|
* Choosing a file, through the real system picker, and still having it after a rotation.
|
||||||
@@ -203,6 +207,8 @@ import org.libremediaconverter.ui.TestTags
|
|||||||
* driven there at all. That is why this gap survived as long as it did.
|
* 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
|
* `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.
|
* 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]
|
* ### Why only the rotation test carries [FailsOnEmulatorApi37]
|
||||||
*
|
*
|
||||||
@@ -251,6 +257,21 @@ class SafPickerRoundTripTest {
|
|||||||
/** Set by the one test that rotates, read by [restoreOrientation]. See its KDoc. */
|
/** Set by the one test that rotates, read by [restoreOrientation]. See its KDoc. */
|
||||||
private var rotated = false
|
private var rotated = false
|
||||||
|
|
||||||
|
/** Counts [MainActivity] creations from the moment [watchForRecreation] is called. */
|
||||||
|
private val recreations = AtomicInteger()
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Counts a rotation's recreation without asking the Activity anything.
|
||||||
|
*
|
||||||
|
* Deliberately not `composeRule.activity`, which resolves through `scenario.onActivity` and so
|
||||||
|
* blocks on the main thread. Polling *that* across a recreation is a plausible reading of the
|
||||||
|
* 20-minute wedges in #122, which would make the obvious barrier the bug it is meant to fix.
|
||||||
|
* The runner's lifecycle monitor is a callback: reading the counter touches no looper.
|
||||||
|
*/
|
||||||
|
private val recreationWatcher = ActivityLifecycleCallback { activity, stage ->
|
||||||
|
if (activity is MainActivity && stage == Stage.CREATED) recreations.incrementAndGet()
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Leave the device the way it was found — and only if this test moved it.
|
* Leave the device the way it was found — and only if this test moved it.
|
||||||
*
|
*
|
||||||
@@ -270,6 +291,7 @@ class SafPickerRoundTripTest {
|
|||||||
*/
|
*/
|
||||||
@After
|
@After
|
||||||
fun restoreOrientation() {
|
fun restoreOrientation() {
|
||||||
|
ActivityLifecycleMonitorRegistry.getInstance().removeLifecycleCallback(recreationWatcher)
|
||||||
if (!rotated) return
|
if (!rotated) return
|
||||||
device.setOrientationNatural()
|
device.setOrientationNatural()
|
||||||
device.unfreezeRotation()
|
device.unfreezeRotation()
|
||||||
@@ -303,9 +325,11 @@ class SafPickerRoundTripTest {
|
|||||||
// The identity hash rather than the Activity itself, so nothing here keeps a destroyed
|
// The identity hash rather than the Activity itself, so nothing here keeps a destroyed
|
||||||
// Activity reachable across the recreation it is being used to detect.
|
// Activity reachable across the recreation it is being used to detect.
|
||||||
val before = System.identityHashCode(composeRule.activity)
|
val before = System.identityHashCode(composeRule.activity)
|
||||||
|
watchForRecreation()
|
||||||
|
|
||||||
device.setOrientationLandscape()
|
device.setOrientationLandscape()
|
||||||
rotated = true
|
rotated = true
|
||||||
|
awaitRecreation()
|
||||||
composeRule.waitForIdle()
|
composeRule.waitForIdle()
|
||||||
|
|
||||||
// Two guards before the assertion that matters, because both of the ways this test could
|
// Two guards before the assertion that matters, because both of the ways this test could
|
||||||
@@ -675,6 +699,36 @@ class SafPickerRoundTripTest {
|
|||||||
* `Condition still not satisfied after 30000 ms` — which names neither the node nor the test.
|
* `Condition still not satisfied after 30000 ms` — which names neither the node nor the test.
|
||||||
* With the description it says which affordance never arrived, which is the whole finding.
|
* With the description it says which affordance never arrived, which is the whole finding.
|
||||||
*/
|
*/
|
||||||
|
/** Starts counting [MainActivity] creations, so [awaitRecreation] can wait for the next one. */
|
||||||
|
private fun watchForRecreation() {
|
||||||
|
recreations.set(0)
|
||||||
|
ActivityLifecycleMonitorRegistry.getInstance().addLifecycleCallback(recreationWatcher)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Waits for the rotation to actually rebuild [MainActivity], which `waitForIdle` does not.
|
||||||
|
*
|
||||||
|
* **This is #122.** `waitForIdle()` waits for the compose hierarchy to settle. Immediately
|
||||||
|
* after a rotation the window manager has accepted but not yet delivered as a configuration
|
||||||
|
* change, the *old* Activity's composition is already idle — so it returns, `composeRule
|
||||||
|
* .activity` still resolves to the old instance, and the guard below reads an unchanged
|
||||||
|
* identity hash. That is the clean `AssertionError` seen on the API 33 gating leg of #217, and
|
||||||
|
* the wedges on #122 are the same race taken the other way: land while the composition is
|
||||||
|
* being torn down and there is nothing coherent for `waitForIdle` to settle on.
|
||||||
|
*
|
||||||
|
* A bounded wait is worth having even if that second half is wrong. It turns a 20-minute
|
||||||
|
* `WEDGE_TIMEOUT` — which costs the leg and names no test — into a fast failure that says which
|
||||||
|
* test and what it was waiting for.
|
||||||
|
*/
|
||||||
|
private fun awaitRecreation() {
|
||||||
|
composeRule.waitUntil(
|
||||||
|
"the rotation did not recreate MainActivity within $RECREATION_TIMEOUT_MS ms",
|
||||||
|
RECREATION_TIMEOUT_MS,
|
||||||
|
) {
|
||||||
|
recreations.get() > 0
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private fun awaitNode(tag: String) {
|
private fun awaitNode(tag: String) {
|
||||||
composeRule.waitUntil("a node tagged $tag exists", APP_TIMEOUT_MS) {
|
composeRule.waitUntil("a node tagged $tag exists", APP_TIMEOUT_MS) {
|
||||||
composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty()
|
composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty()
|
||||||
@@ -703,6 +757,15 @@ class SafPickerRoundTripTest {
|
|||||||
*/
|
*/
|
||||||
const val REOPENED_TIMEOUT_MS = 10_000L
|
const val REOPENED_TIMEOUT_MS = 10_000L
|
||||||
|
|
||||||
|
/**
|
||||||
|
* How long a rotation is given to destroy and rebuild the Activity.
|
||||||
|
*
|
||||||
|
* Generous against the API 33 and 34 emulators #122 was measured on, where the rotation is
|
||||||
|
* slow enough for the gap this bound exists to cover to be observable at all — and still
|
||||||
|
* two orders of magnitude inside the 1200 s `WEDGE_TIMEOUT` it replaces.
|
||||||
|
*/
|
||||||
|
const val RECREATION_TIMEOUT_MS = 15_000L
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* How long the app is given to take the window focus back after a back press.
|
* How long the app is given to take the window focus back after a back press.
|
||||||
*
|
*
|
||||||
|
|||||||
+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
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -3,6 +3,7 @@ package org.libremediaconverter
|
|||||||
import android.app.Application
|
import android.app.Application
|
||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.Job
|
||||||
import kotlinx.coroutines.SupervisorJob
|
import kotlinx.coroutines.SupervisorJob
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
import org.libremediaconverter.convert.OutputPublisher
|
import org.libremediaconverter.convert.OutputPublisher
|
||||||
@@ -17,14 +18,36 @@ import org.libremediaconverter.convert.OutputPublisher
|
|||||||
* ever becomes a `Converted` state, or a `reset()`'s delete is cancelled along with the
|
* ever becomes a `Converted` state, or a `reset()`'s delete is cancelled along with the
|
||||||
* Activity. Process start is the one moment those leftovers are reliably observable.
|
* Activity. Process start is the one moment those leftovers are reliably observable.
|
||||||
*/
|
*/
|
||||||
class LibreMediaConverterApp : Application() {
|
open class LibreMediaConverterApp : Application() {
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Deliberately process-lifetime and never cancelled: the work it carries is a single
|
* Deliberately process-lifetime and never cancelled: the work it carries is a single
|
||||||
* short task that should outlive nothing in particular and be interrupted by nothing.
|
* short task that should outlive nothing in particular and be interrupted by nothing.
|
||||||
* A `SupervisorJob` so a failure here could never take a sibling down with it.
|
* A `SupervisorJob` so a failure here could never take a sibling down with it.
|
||||||
|
*
|
||||||
|
* **`protected open` for #159.** Robolectric builds an `Application` for every test that asks
|
||||||
|
* for one, so on the JVM this is not one background sweep but one *per test* — all of them on
|
||||||
|
* `Dispatchers.IO`, all touching the same `cacheDir`, none of them joined by anything. That is
|
||||||
|
* a race against any test asserting about a file under `conversions/`, and it grew with the
|
||||||
|
* suite: wave 4 added ten Robolectric classes and took it from CI-only to roughly one local run
|
||||||
|
* in six. The JVM suite substitutes a scope that runs the sweep inline — see
|
||||||
|
* `app/src/test/resources/robolectric.properties` and `TestLibreMediaConverterApp`.
|
||||||
|
*
|
||||||
|
* A constructor parameter would be the ordinary way to inject this and is not available: the
|
||||||
|
* framework builds this class, so the seam has to be a member.
|
||||||
*/
|
*/
|
||||||
private val appScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
|
protected open val sweepScope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The sweep [onCreate] last started, so a caller that needs it finished can wait for it.
|
||||||
|
*
|
||||||
|
* Nothing in production reads this — process start does not wait for its own housekeeping. It
|
||||||
|
* exists because the alternative for a test is a timed poll, and a poll cannot tell "the sweep
|
||||||
|
* has not run yet" from "the sweep ran and did nothing".
|
||||||
|
*/
|
||||||
|
@Volatile
|
||||||
|
var startupSweep: Job? = null
|
||||||
|
private set
|
||||||
|
|
||||||
override fun onCreate() {
|
override fun onCreate() {
|
||||||
super.onCreate()
|
super.onCreate()
|
||||||
@@ -53,6 +76,6 @@ class LibreMediaConverterApp : Application() {
|
|||||||
//
|
//
|
||||||
// sweepStaging() also re-reads each timestamp immediately before deleting, which
|
// sweepStaging() also re-reads each timestamp immediately before deleting, which
|
||||||
// closes the window between listing the directory and acting on the listing.
|
// closes the window between listing the directory and acting on the listing.
|
||||||
appScope.launch { OutputPublisher(this@LibreMediaConverterApp).sweepStaging() }
|
startupSweep = sweepScope.launch { OutputPublisher(this@LibreMediaConverterApp).sweepStaging() }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,7 +5,6 @@ import android.net.Uri
|
|||||||
import android.util.Log
|
import android.util.Log
|
||||||
import com.arthenica.ffmpegkit.FFmpegKit
|
import com.arthenica.ffmpegkit.FFmpegKit
|
||||||
import com.arthenica.ffmpegkit.FFmpegKitConfig
|
import com.arthenica.ffmpegkit.FFmpegKitConfig
|
||||||
import com.arthenica.ffmpegkit.ReturnCode
|
|
||||||
import kotlinx.coroutines.suspendCancellableCoroutine
|
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||||
import org.libremediaconverter.convert.ConcatJoiner
|
import org.libremediaconverter.convert.ConcatJoiner
|
||||||
import org.libremediaconverter.convert.MediaProbe
|
import org.libremediaconverter.convert.MediaProbe
|
||||||
@@ -66,16 +65,16 @@ class ConcatEngine(private val context: Context) : ConcatJoiner {
|
|||||||
private suspend fun execute(args: List<String>) = suspendCancellableCoroutine { cont ->
|
private suspend fun execute(args: List<String>) = suspendCancellableCoroutine { cont ->
|
||||||
Log.i(TAG, "ffmpeg ${args.joinToString(" ")}")
|
Log.i(TAG, "ffmpeg ${args.joinToString(" ")}")
|
||||||
val session = FFmpegKit.executeWithArgumentsAsync(args.toTypedArray()) { completed ->
|
val session = FFmpegKit.executeWithArgumentsAsync(args.toTypedArray()) { completed ->
|
||||||
val rc = completed.getReturnCode()
|
val outcome = sessionOutcome(
|
||||||
when {
|
rc = completed.getReturnCode(),
|
||||||
ReturnCode.isSuccess(rc) -> cont.resume(Unit)
|
prefix = "Joining",
|
||||||
ReturnCode.isCancel(rc) -> cont.cancel()
|
failStackTrace = { completed.getFailStackTrace() },
|
||||||
else -> cont.resumeWithException(
|
logTail = { completed.getAllLogsAsString(LOG_TAIL_LIMIT) },
|
||||||
FFmpegEngine.FFmpegException(
|
|
||||||
"Joining failed (${rc?.value}): " +
|
|
||||||
completed.getAllLogsAsString(LOG_TAIL_LIMIT).orEmpty(),
|
|
||||||
),
|
|
||||||
)
|
)
|
||||||
|
when (outcome) {
|
||||||
|
SessionOutcome.Success -> cont.resume(Unit)
|
||||||
|
SessionOutcome.Cancelled -> cont.cancel()
|
||||||
|
is SessionOutcome.Failed -> cont.resumeWithException(FFmpegEngine.FFmpegException(outcome.message))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
cont.invokeOnCancellation { FFmpegKit.cancel(session.getSessionId()) }
|
cont.invokeOnCancellation { FFmpegKit.cancel(session.getSessionId()) }
|
||||||
|
|||||||
@@ -4,7 +4,6 @@ import android.util.Log
|
|||||||
import com.arthenica.ffmpegkit.FFmpegKit
|
import com.arthenica.ffmpegkit.FFmpegKit
|
||||||
import com.arthenica.ffmpegkit.FFmpegKitConfig
|
import com.arthenica.ffmpegkit.FFmpegKitConfig
|
||||||
import com.arthenica.ffmpegkit.Level
|
import com.arthenica.ffmpegkit.Level
|
||||||
import com.arthenica.ffmpegkit.ReturnCode
|
|
||||||
import kotlinx.coroutines.suspendCancellableCoroutine
|
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||||
import org.libremediaconverter.convert.SoftwareTranscoder
|
import org.libremediaconverter.convert.SoftwareTranscoder
|
||||||
import org.libremediaconverter.model.ConversionRequest
|
import org.libremediaconverter.model.ConversionRequest
|
||||||
@@ -51,19 +50,16 @@ class FFmpegEngine : SoftwareTranscoder {
|
|||||||
val session = FFmpegKit.executeWithArgumentsAsync(
|
val session = FFmpegKit.executeWithArgumentsAsync(
|
||||||
args.toTypedArray(),
|
args.toTypedArray(),
|
||||||
{ completed ->
|
{ completed ->
|
||||||
val rc = completed.getReturnCode()
|
val outcome = sessionOutcome(
|
||||||
when {
|
rc = completed.getReturnCode(),
|
||||||
ReturnCode.isSuccess(rc) -> cont.resume(Unit)
|
prefix = "FFmpeg",
|
||||||
ReturnCode.isCancel(rc) ->
|
failStackTrace = { completed.getFailStackTrace() },
|
||||||
cont.cancel()
|
logTail = { completed.getAllLogsAsString(LOG_TAIL_LIMIT) },
|
||||||
else -> cont.resumeWithException(
|
|
||||||
FFmpegException(
|
|
||||||
"FFmpeg failed (${rc?.value}): " +
|
|
||||||
completed.getFailStackTrace().orEmpty().ifBlank {
|
|
||||||
completed.getAllLogsAsString(LOG_TAIL_LIMIT).orEmpty()
|
|
||||||
},
|
|
||||||
),
|
|
||||||
)
|
)
|
||||||
|
when (outcome) {
|
||||||
|
SessionOutcome.Success -> cont.resume(Unit)
|
||||||
|
SessionOutcome.Cancelled -> cont.cancel()
|
||||||
|
is SessionOutcome.Failed -> cont.resumeWithException(FFmpegException(outcome.message))
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
{ log -> Log.d(TAG, log.message.trimEnd()) },
|
{ log -> Log.d(TAG, log.message.trimEnd()) },
|
||||||
|
|||||||
@@ -0,0 +1,54 @@
|
|||||||
|
package org.libremediaconverter.ffmpeg
|
||||||
|
|
||||||
|
import com.arthenica.ffmpegkit.ReturnCode
|
||||||
|
|
||||||
|
/**
|
||||||
|
* What a finished FFmpegKit session means, as a function of its return code.
|
||||||
|
*
|
||||||
|
* Both engines had their own copy of this `when`, twelve lines apart in two files, and the copies
|
||||||
|
* had drifted: [FFmpegEngine] preferred the fail stack trace and fell back to the log tail, while
|
||||||
|
* [ConcatEngine] only ever read the log tail. Neither was tested — both live inside a callback
|
||||||
|
* handed to `FFmpegKit`, which does not run on the JVM — so the divergence was invisible.
|
||||||
|
*
|
||||||
|
* #203 decided to unify on the stack trace, so a join failure now carries the diagnostics a
|
||||||
|
* conversion failure always did. The *prefix* stays per-engine: unifying the strategy must not
|
||||||
|
* unify the sentence, since "FFmpeg failed" and "Joining failed" describe different jobs.
|
||||||
|
*/
|
||||||
|
internal sealed interface SessionOutcome {
|
||||||
|
|
||||||
|
/** rc 0. The suspension resumes normally. */
|
||||||
|
data object Success : SessionOutcome
|
||||||
|
|
||||||
|
/** rc 255. The suspension is cancelled rather than failed — the user asked for this. */
|
||||||
|
data object Cancelled : SessionOutcome
|
||||||
|
|
||||||
|
/** Anything else, with the sentence the user is shown. */
|
||||||
|
data class Failed(val message: String) : SessionOutcome
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Maps a return code onto the outcome, and builds the failure sentence when there is one.
|
||||||
|
*
|
||||||
|
* **The two message parts arrive as lambdas, deliberately.** `getAllLogsAsString` and
|
||||||
|
* `getFailStackTrace` are calls onto a native session, and only the failure arm needs either. Taking
|
||||||
|
* them by value would put both on the happy path of every successful conversion, which is a cost the
|
||||||
|
* shape this replaced did not have — the old code read them inside the `else` branch. That is the
|
||||||
|
* same reason [org.libremediaconverter.codec.AndroidDeviceCodecs.capabilitiesFrom] takes a
|
||||||
|
* `Sequence`: a seam should not change what runs when.
|
||||||
|
*
|
||||||
|
* A null [rc] is a real input rather than a defensive one — `getReturnCode()` is nullable, and a
|
||||||
|
* session killed before it reported anything has none. It is neither success nor cancellation, so
|
||||||
|
* it fails, and the sentence says `null` where the number would be.
|
||||||
|
*/
|
||||||
|
internal fun sessionOutcome(
|
||||||
|
rc: ReturnCode?,
|
||||||
|
prefix: String,
|
||||||
|
failStackTrace: () -> String?,
|
||||||
|
logTail: () -> String?,
|
||||||
|
): SessionOutcome = when {
|
||||||
|
ReturnCode.isSuccess(rc) -> SessionOutcome.Success
|
||||||
|
ReturnCode.isCancel(rc) -> SessionOutcome.Cancelled
|
||||||
|
else -> SessionOutcome.Failed(
|
||||||
|
"$prefix failed (${rc?.value}): " + failStackTrace().orEmpty().ifBlank { logTail().orEmpty() },
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -1,8 +1,8 @@
|
|||||||
package org.libremediaconverter
|
package org.libremediaconverter
|
||||||
|
|
||||||
import org.junit.Assert.assertEquals
|
import kotlinx.coroutines.runBlocking
|
||||||
|
import org.junit.Assert.assertNotNull
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
import org.junit.Assert.fail
|
|
||||||
import org.junit.Before
|
import org.junit.Before
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
import org.junit.runner.RunWith
|
import org.junit.runner.RunWith
|
||||||
@@ -10,7 +10,6 @@ import org.libremediaconverter.convert.StagingSweep
|
|||||||
import org.robolectric.RobolectricTestRunner
|
import org.robolectric.RobolectricTestRunner
|
||||||
import org.robolectric.RuntimeEnvironment
|
import org.robolectric.RuntimeEnvironment
|
||||||
import java.io.File
|
import java.io.File
|
||||||
import java.util.concurrent.TimeUnit
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* That process start actually sweeps.
|
* That process start actually sweeps.
|
||||||
@@ -23,8 +22,19 @@ import java.util.concurrent.TimeUnit
|
|||||||
* output ever became a `Converted` state, a `reset()` whose delete was cancelled with the Activity.
|
* output ever became a `Converted` state, a `reset()` whose delete was cancelled with the Activity.
|
||||||
*
|
*
|
||||||
* `onCreate()` is called again rather than a second Application being built: it is what the
|
* `onCreate()` is called again rather than a second Application being built: it is what the
|
||||||
* framework calls at process start, the scope it launches on is already there, and the first test
|
* framework calls at process start, and the scope it launches on is already there.
|
||||||
* below is what pins that the framework calls it on *this* class.
|
*
|
||||||
|
* **What this class stopped covering in #159, deliberately.** It used to open by asserting that
|
||||||
|
* `RuntimeEnvironment.getApplication()` is a [LibreMediaConverterApp] — that the manifest's
|
||||||
|
* `android:name` points here, so the sweep is code that actually runs. That assertion cannot exist
|
||||||
|
* on the JVM any more: `robolectric.properties` now names [TestLibreMediaConverterApp] for the
|
||||||
|
* whole suite, and an `application=` override replaces the manifest rather than being checked
|
||||||
|
* against it — `applicationInfo.className` reports the override too, measured. So the manifest is
|
||||||
|
* not merely unasserted here, it is unobservable from this source set, and a rewritten version of
|
||||||
|
* that test would have asserted the override against itself. **The manifest link is a device-only
|
||||||
|
* guarantee now**, and it was traded knowingly for the race that override fixes. The cast in
|
||||||
|
* [setUp] still fails if [TestLibreMediaConverterApp] stops extending the real class, which is a
|
||||||
|
* smaller claim than the one withdrawn.
|
||||||
*/
|
*/
|
||||||
@RunWith(RobolectricTestRunner::class)
|
@RunWith(RobolectricTestRunner::class)
|
||||||
class AppStartSweepTest {
|
class AppStartSweepTest {
|
||||||
@@ -34,17 +44,35 @@ class AppStartSweepTest {
|
|||||||
|
|
||||||
@Before
|
@Before
|
||||||
fun setUp() {
|
fun setUp() {
|
||||||
// The cast is an assertion in itself: Robolectric builds the Application named in the
|
|
||||||
// merged manifest, so this fails if `android:name` ever stops pointing here -- in which
|
|
||||||
// case the sweep below would be perfectly correct code that never runs.
|
|
||||||
app = RuntimeEnvironment.getApplication() as LibreMediaConverterApp
|
app = RuntimeEnvironment.getApplication() as LibreMediaConverterApp
|
||||||
stagingDir = File(app.cacheDir, "conversions").apply { mkdirs() }
|
stagingDir = File(app.cacheDir, "conversions").apply { mkdirs() }
|
||||||
stagingDir.listFiles()?.forEach { it.delete() }
|
stagingDir.listFiles()?.forEach { it.delete() }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The property the whole substitution exists for, asserted directly rather than waited on.
|
||||||
|
*
|
||||||
|
* #159 is not "the sweep is slow", it is "the sweep is still running while some later test
|
||||||
|
* reads the directory". [TestLibreMediaConverterApp] answers that by finishing the sweep before
|
||||||
|
* `onCreate()` returns, and this is the only place that claim is checked -- every other test in
|
||||||
|
* the suite benefits from it silently and would go back to racing without saying why.
|
||||||
|
*
|
||||||
|
* Deterministic in the direction that matters: `Dispatchers.Unconfined` runs a `launch` whose
|
||||||
|
* body never suspends to completion inline, so this cannot flake green-to-red. Putting the test
|
||||||
|
* app back on `Dispatchers.IO` makes it a race that the assertion loses essentially every time,
|
||||||
|
* which is what a six-run suite comparison could not show -- at the rate #159 was observed at,
|
||||||
|
* a clean six-run arm is a coin flip.
|
||||||
|
*/
|
||||||
@Test
|
@Test
|
||||||
fun `the application the manifest starts is the one that sweeps`() {
|
fun `the sweep is finished before onCreate returns`() {
|
||||||
assertEquals(LibreMediaConverterApp::class.java, RuntimeEnvironment.getApplication().javaClass)
|
app.onCreate()
|
||||||
|
|
||||||
|
val sweep = app.startupSweep
|
||||||
|
assertNotNull("onCreate() started no sweep", sweep)
|
||||||
|
assertTrue(
|
||||||
|
"the JVM suite's sweep outlived onCreate(), so it is in flight during test bodies again",
|
||||||
|
sweep?.isCompleted == true,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -64,35 +92,23 @@ class AppStartSweepTest {
|
|||||||
|
|
||||||
app.onCreate()
|
app.onCreate()
|
||||||
|
|
||||||
awaitGone(abandoned)
|
// Joined rather than polled. `onCreate` publishes the sweep it started, so this waits for
|
||||||
|
// that exact sweep -- where a timed poll could not tell "swept" from "not started yet", and
|
||||||
|
// answered the second case by failing after ten seconds.
|
||||||
|
val sweep = app.startupSweep
|
||||||
|
assertNotNull("onCreate() started no sweep to wait for", sweep)
|
||||||
|
runBlocking { sweep?.join() }
|
||||||
|
|
||||||
|
assertTrue("process start left ${abandoned.name} in staging; nothing swept it", !abandoned.exists())
|
||||||
// The other half, and the one that says the sweep is a sweep rather than a
|
// The other half, and the one that says the sweep is a sweep rather than a
|
||||||
// `clearStaging()`: the directory is shared by the convert tab, the join tab and
|
// `clearStaging()`: the directory is shared by the convert tab, the join tab and
|
||||||
// ConcatEngine's list file, so deleting everything could take a file from a running job.
|
// ConcatEngine's list file, so deleting everything could take a file from a running job.
|
||||||
assertTrue("a file written moments ago belongs to a live job", live.exists())
|
assertTrue("a file written moments ago belongs to a live job", live.exists())
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Waits for [file] to be deleted.
|
|
||||||
*
|
|
||||||
* The sweep runs on `Dispatchers.IO`, deliberately: it lists a directory and stats every entry
|
|
||||||
* on the path that decides how long the launcher icon stays unresponsive. So there is nothing
|
|
||||||
* to join, and the wait is a bounded poll — long enough for a directory listing, short enough
|
|
||||||
* that a sweep which never happens fails rather than hangs.
|
|
||||||
*/
|
|
||||||
private fun awaitGone(file: File) {
|
|
||||||
val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(AWAIT_TIMEOUT_SECONDS)
|
|
||||||
while (System.nanoTime() < deadline) {
|
|
||||||
if (!file.exists()) return
|
|
||||||
Thread.sleep(POLL_INTERVAL_MS)
|
|
||||||
}
|
|
||||||
fail("process start left ${file.name} in staging; nothing swept it")
|
|
||||||
}
|
|
||||||
|
|
||||||
private fun stagedFile(name: String): File = File(stagingDir, name).apply { writeBytes(ByteArray(4096)) }
|
private fun stagedFile(name: String): File = File(stagingDir, name).apply { writeBytes(ByteArray(4096)) }
|
||||||
|
|
||||||
private companion object {
|
private companion object {
|
||||||
const val ONE_MINUTE_MS = 60L * 1000
|
const val ONE_MINUTE_MS = 60L * 1000
|
||||||
const val AWAIT_TIMEOUT_SECONDS = 10L
|
|
||||||
const val POLL_INTERVAL_MS = 5L
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,28 @@
|
|||||||
|
package org.libremediaconverter
|
||||||
|
|
||||||
|
import kotlinx.coroutines.CoroutineScope
|
||||||
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.SupervisorJob
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The [LibreMediaConverterApp] the JVM suite runs, differing from it in exactly one thing: the
|
||||||
|
* startup sweep runs inline on the thread that builds the Application instead of on
|
||||||
|
* `Dispatchers.Unconfined`.
|
||||||
|
*
|
||||||
|
* **This is #159.** Robolectric builds an `Application` per test class that asks for one, and each
|
||||||
|
* one launches a sweep over the shared `<cacheDir>/conversions/`. Nothing joins them, so a test
|
||||||
|
* asserting about a staged file is racing however many sweeps the classes before it left in
|
||||||
|
* flight — `OutputPublisherStagingTest` being the one that lost, at roughly one local run in six
|
||||||
|
* once wave 4 added ten more Robolectric classes. Making the sweep finish before `onCreate()`
|
||||||
|
* returns removes the race for every test at once rather than asking each to opt in; 27 of the
|
||||||
|
* suite's 58 Robolectric classes touch that directory, so opting in was not a real option.
|
||||||
|
*
|
||||||
|
* `Dispatchers.Unconfined` is what makes it inline: `sweepStaging()` is a plain function, so an
|
||||||
|
* `Unconfined` `launch` runs it to completion before returning. The `SupervisorJob` is kept so this
|
||||||
|
* differs from production in the dispatcher alone — a sweep that throws is logged and swallowed
|
||||||
|
* here exactly as it is there, rather than taking Application construction down with it and failing
|
||||||
|
* every test in the class for an unrelated reason.
|
||||||
|
*/
|
||||||
|
class TestLibreMediaConverterApp : LibreMediaConverterApp() {
|
||||||
|
override val sweepScope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Unconfined)
|
||||||
|
}
|
||||||
@@ -122,24 +122,25 @@ class OutputPublisherStagingTest {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Makes `cacheDir/conversions` a regular file, which is the whole precondition of the test
|
* Makes `cacheDir/conversions` a regular file, which is the whole precondition of the test
|
||||||
* above -- and does it in a loop, because a single delete-then-write loses a race that CI
|
* above -- and does it in a loop, because a single delete-then-write once lost a race that CI
|
||||||
* caught and this machine does not reproduce.
|
* caught and this machine did not reproduce.
|
||||||
*
|
*
|
||||||
* `LibreMediaConverterApp.onCreate` ends with
|
* **That race is closed at the source as of #159, and the loop is kept anyway.**
|
||||||
* `appScope.launch { OutputPublisher(...).sweepStaging() }` on `Dispatchers.IO`, and
|
* `LibreMediaConverterApp.onCreate` launched its staging sweep on `Dispatchers.IO`, and
|
||||||
* `sweepStaging` reads `stagingDir`, whose getter calls `mkdirs()`. Robolectric instantiates
|
* `sweepStaging` reads `stagingDir`, whose getter calls `mkdirs()`. Robolectric builds an
|
||||||
* the application for every test that asks for one, so that background `mkdirs()` is in flight
|
* application for every test class that asks for one, so that background `mkdirs()` was in
|
||||||
* across the whole suite, on a thread the paused main looper does not control. Between deleting
|
* flight across the whole suite, on a thread the paused main looper does not control. Between
|
||||||
* this path and writing it there is a window where the path does not exist and that `mkdirs()`
|
* deleting this path and writing it there is a window where the path does not exist and that
|
||||||
* can win, which is `FileNotFoundException: ... (Is a directory)` out of `writeBytes` -- run
|
* `mkdirs()` could win -- `FileNotFoundException: ... (Is a directory)` out of `writeBytes`,
|
||||||
* 33069641674 on #149, once, against 468 tests that pass here.
|
* run 33069641674 on #149, once, against 468 tests that passed here. The JVM suite now runs
|
||||||
|
* `TestLibreMediaConverterApp`, whose sweep finishes before `onCreate()` returns, so nothing is
|
||||||
|
* sweeping while a test body runs.
|
||||||
*
|
*
|
||||||
* Retrying closes it rather than narrowing it, because the race is not symmetric: `mkdirs()`
|
* The loop stays because it is what would catch that substitution being undone. Without it the
|
||||||
* fails on an existing regular file, so the invariant only has to survive being *established*.
|
* regression returns as this one class failing rarely on CI -- the exact shape that took #159
|
||||||
* Once a write lands, nothing in the suite can turn this back into a directory.
|
* from a single run on #149 to a wave-4 flake before anyone chased it. Retrying closes the
|
||||||
*
|
* window rather than narrowing it, because the race is not symmetric: `mkdirs()` fails on an
|
||||||
* The wider problem -- application-scope IO work racing every Robolectric test that shares
|
* existing regular file, so the invariant only has to survive being *established*.
|
||||||
* `cacheDir` -- is #159, and is deliberately not fixed here.
|
|
||||||
*/
|
*/
|
||||||
private fun stagingPathAsRegularFile(): File {
|
private fun stagingPathAsRegularFile(): File {
|
||||||
val stagingPath = File(cacheDir, "conversions")
|
val stagingPath = File(cacheDir, "conversions")
|
||||||
|
|||||||
@@ -0,0 +1,127 @@
|
|||||||
|
package org.libremediaconverter.ffmpeg
|
||||||
|
|
||||||
|
import com.arthenica.ffmpegkit.ReturnCode
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertTrue
|
||||||
|
import org.junit.Test
|
||||||
|
|
||||||
|
/**
|
||||||
|
* What a finished FFmpegKit session means, for both engines at once.
|
||||||
|
*
|
||||||
|
* `FFmpegEngine` and `ConcatEngine` each carried their own copy of this `when`, and the copies had
|
||||||
|
* drifted: one preferred the fail stack trace and fell back to the log tail, the other only ever
|
||||||
|
* read the log tail. Neither was tested, because both live inside a callback handed to `FFmpegKit`,
|
||||||
|
* which does not run on the JVM — so nothing could see that the two disagreed.
|
||||||
|
*
|
||||||
|
* **JVM-safe, verified rather than assumed.** `javap` over the committed AAR's runtime jar shows
|
||||||
|
* `ReturnCode(int)` as a plain public constructor with `SUCCESS`/`CANCEL` int constants and pure
|
||||||
|
* static `isSuccess`/`isCancel`; its `<clinit>` is constant initialisation and loads no native
|
||||||
|
* library.
|
||||||
|
*
|
||||||
|
* The unification is #203's decision, so the tests pin it as one: a join failure now carries the
|
||||||
|
* stack trace a conversion failure always did, while the two prefixes stay distinct.
|
||||||
|
*/
|
||||||
|
class SessionOutcomeTest {
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a return code of zero is success`() {
|
||||||
|
assertEquals(SessionOutcome.Success, outcome(ReturnCode(ReturnCode.SUCCESS)))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Cancellation is a separate outcome from failure, and the distinction is the point: the engines
|
||||||
|
* resume the continuation *cancelled* rather than exceptionally, so a user who pressed Cancel
|
||||||
|
* does not get an error card.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `a return code of 255 is a cancellation, not a failure`() {
|
||||||
|
assertEquals(SessionOutcome.Cancelled, outcome(ReturnCode(ReturnCode.CANCEL)))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `any other return code fails, and the sentence carries the number`() {
|
||||||
|
val failed = outcome(ReturnCode(1), stackTrace = "boom") as SessionOutcome.Failed
|
||||||
|
|
||||||
|
assertTrue("the code belongs in the message, got: ${failed.message}", failed.message.contains("(1)"))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The half that was different between the two engines before #203, now the same in both.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `the stack trace is preferred over the log tail`() {
|
||||||
|
val failed = outcome(ReturnCode(1), stackTrace = "the real cause", logTail = "…noise…")
|
||||||
|
as SessionOutcome.Failed
|
||||||
|
|
||||||
|
assertTrue(failed.message.contains("the real cause"))
|
||||||
|
assertTrue("the log tail must not be appended as well", !failed.message.contains("noise"))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a blank stack trace falls back to the log tail`() {
|
||||||
|
val blank = outcome(ReturnCode(1), stackTrace = " ", logTail = "the last few lines") as SessionOutcome.Failed
|
||||||
|
val absent = outcome(ReturnCode(1), stackTrace = null, logTail = "the last few lines") as SessionOutcome.Failed
|
||||||
|
|
||||||
|
assertTrue(blank.message.contains("the last few lines"))
|
||||||
|
assertTrue("a null stack trace is a blank one", absent.message.contains("the last few lines"))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Both sources empty still has to produce a sentence. A message ending in a dangling colon is
|
||||||
|
* thin, but it is what the user gets when FFmpeg said nothing at all, and it must not be an
|
||||||
|
* exception on the way to the screen.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `a failure with nothing to say still names the code`() {
|
||||||
|
val failed = outcome(ReturnCode(1), stackTrace = null, logTail = null) as SessionOutcome.Failed
|
||||||
|
|
||||||
|
assertEquals("FFmpeg failed (1): ", failed.message)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* `getReturnCode()` is nullable and a session killed before it reported anything has none.
|
||||||
|
* Neither success nor cancellation, so it fails — and the sentence says so rather than throwing.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `a session with no return code at all fails`() {
|
||||||
|
val failed = outcome(null, logTail = "whatever was logged") as SessionOutcome.Failed
|
||||||
|
|
||||||
|
assertTrue("got: ${failed.message}", failed.message.startsWith("FFmpeg failed (null): "))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Unifying the *strategy* must not unify the *sentence*: the two engines describe different
|
||||||
|
* jobs, and a join that reports "FFmpeg failed" is a worse message than the one it replaced.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `each engine keeps its own prefix`() {
|
||||||
|
val join = sessionOutcome(ReturnCode(1), "Joining", { "cause" }, { null }) as SessionOutcome.Failed
|
||||||
|
|
||||||
|
assertTrue(join.message.startsWith("Joining failed (1): "))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Neither message source is read unless the outcome is a failure.
|
||||||
|
*
|
||||||
|
* They are calls onto a native session, and reading them on the happy path is work every
|
||||||
|
* successful conversion would do for nothing — which the shape this replaced did not, since it
|
||||||
|
* read them inside the `else` branch. That is why the parameters are lambdas, and this is what
|
||||||
|
* would notice if they stopped being.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `a session that succeeded reads neither the stack trace nor the log`() {
|
||||||
|
var reads = 0
|
||||||
|
fun counted(): String? {
|
||||||
|
reads++
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
|
||||||
|
sessionOutcome(ReturnCode(ReturnCode.SUCCESS), "FFmpeg", ::counted, ::counted)
|
||||||
|
sessionOutcome(ReturnCode(ReturnCode.CANCEL), "FFmpeg", ::counted, ::counted)
|
||||||
|
|
||||||
|
assertEquals("neither source may be touched unless the session failed", 0, reads)
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun outcome(rc: ReturnCode?, stackTrace: String? = null, logTail: String? = null) =
|
||||||
|
sessionOutcome(rc, "FFmpeg", { stackTrace }, { logTail })
|
||||||
|
}
|
||||||
@@ -10,3 +10,9 @@
|
|||||||
# Set here rather than in a @Config on each class so a later Robolectric test does not have
|
# Set here rather than in a @Config on each class so a later Robolectric test does not have
|
||||||
# to rediscover it. Remove it once Robolectric ships an android-all jar for 37.
|
# to rediscover it. Remove it once Robolectric ships an android-all jar for 37.
|
||||||
sdk=36
|
sdk=36
|
||||||
|
|
||||||
|
# Every test gets TestLibreMediaConverterApp, whose only difference from the real one is that the
|
||||||
|
# startup sweep runs inline rather than on Dispatchers.IO. Set suite-wide because the race it fixes
|
||||||
|
# (#159) is suite-wide: any class that builds an Application leaves a sweep of the shared staging
|
||||||
|
# directory in flight for whatever runs next. TestLibreMediaConverterApp explains the choice.
|
||||||
|
application=org.libremediaconverter.TestLibreMediaConverterApp
|
||||||
|
|||||||
@@ -0,0 +1,327 @@
|
|||||||
|
# E2E-read findings
|
||||||
|
|
||||||
|
**Status:** six findings, none fixed, 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.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 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 | **fix the sentence** |
|
||||||
|
| 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** |
|
||||||
|
|
||||||
|
**Five of the six 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 |
|
||||||
|
|---|---|
|
||||||
|
| **#223** | `HardwareFallbackTest` never attempts the hardware path on any emulator leg |
|
||||||
|
| **#224** | Cancelling a *running* native session, in any of the three engines |
|
||||||
|
| **#225** | No `content://` input has reached a *successful* conversion — the ffkitsaf bridge |
|
||||||
|
| **#226** | `OutputPublisher.publish` against a real `DocumentsProvider`, and the SAF premise it rests on |
|
||||||
|
| **#227** | The notification's Cancel action has never been fired |
|
||||||
|
| **#228** | `encodesFlacLosslessAudio` and `encodesOpus` pass on any non-empty file |
|
||||||
|
| **#229** | FFmpeg's progress percentage is computed everywhere and asserted nowhere |
|
||||||
|
| **#230** | *(spike)* whether a running conversion's process can be killed under instrumentation |
|
||||||
|
|
||||||
|
**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
|
`reportDeviceEncoderCapabilities`, has no such guard and runs. A level reporting 0 skipped
|
||||||
would mean someone had staged sample files, not that something improved.
|
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
|
### What the sweep adds, and what it does not
|
||||||
|
|
||||||
**The renderer rule held four more times.** No boot log contains the string
|
**The renderer rule held four more times.** No boot log contains the string
|
||||||
|
|||||||
Reference in New Issue
Block a user