Compare commits

..
Author SHA1 Message Date
JMR-devandClaude Opus 5 da344b0fe4 Re-measure after wave 3, and write down the two kinds of gap it had to separate
92.8% line (2183/2352), 81.3% branch (1091/1342), 584 JVM tests in 87 classes, measured
2026-09-02 on the tree this branch creates rather than quoted from a PR body.

The shape of the wave is worth more than the number, and it is different from the two
before it. Waves 1 and 2 were finding uncovered code; by wave 3 there was little of that
left, so the gaps had to be sorted before any test was written. Coverage gaps -- filtered
to sites where JaCoCo reports mi > 0, which is what separates a real gap from a partial
branch on a compound condition, and which cut the candidate list roughly in half. And
assertion gaps, where JaCoCo is green and nothing checks the answer: MainActivity's rail
and bottom bar were both executed and transposing them passed the entire suite, as did
swapping the two progress-notification strings and swapping Content's two destinations.
No coverage number would have found any of the three.

Naming the required mutation per ticket earned its keep three times, each recorded with
what the weak assertion actually was. Also recorded: a green mutation is only evidence
when the mutation is a real change -- one classify reordering was semantically equivalent
for every reachable input, and a bad mutation and a weak test look identical in the output.

Two entries came back as not gaps, which is a result rather than a shortfall:
ContainerCapabilities:282's exclude filter cannot drop anything, and probeForConcat's
catch arm is unreachable on this runtime -- Robolectric's MediaExtractor never throws from
setDataSource, measured across four input shapes.

Both denominators moved, in opposite directions and for different reasons, so they are
stated rather than folded into the percentage: 1340 -> 1342 branches from MediaProbe.merge,
2348 -> 2352 lines from the ConcatJoiner interface. Neither is new untested code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 22:09:35 -05:00
JMR-devandClaude Opus 5 aa7e1d8b01 C1 (#176): give ConcatWorker the seam ConversionWorker always had, and test what was behind it
ConcatWorker constructed ConcatEngine in place while ConversionWorker reached its engines
through ConversionDependencies. That asymmetry is the whole reason one worker had a tested
failure path and the other had none: everything past setForeground was untested on *every*
source set, JVM and device alike.

The repo had already measured the cost and written it down. PerJobStagingTest's KDoc
records that **reverting ConcatWorker to a constant staging name left all 257 tests
green**, because nothing could reach the line that names the file. RefusedJobTest says it
from the other side -- "the next thing past the count guard is ConcatEngine, which is
native". That mutation is red now.

The seam is `ConversionDependencies.concat: (Context) -> ConcatJoiner`, beside .hardware
and .software. ConcatEngine implements the interface; its Result type stays nested in the
implementation, because moving it would touch every call site to buy nothing -- what a
test needs is the ability to not run FFmpeg, and that is the method, not the type.

Five tests, and the mutations that hold them:

  the engine's own reason reaches the user   replace e.message with the generic string
  a failure with no message still says one   drop the ?: GENERIC_FAILURE_MESSAGE fallback
  a failed join deletes its partial          drop staged.delete() from the catch
  no input array at all is refused           swap in TOO_FEW_INPUTS_MESSAGE
  a join reports its own staged file         revert to the constant staging name

The delete test was vacuous on its first draft and the mutation caught it: it scanned the
staging directory for a "join-" prefix that StagingNames.forJob does not produce -- it
names files <jobId>.<ext> -- so the assertion was trivially true. Rewritten to assert
against the handle the joiner was actually given.

Two small fixes ride along, both the repo's own conventions rather than new opinions.
"No input files." becomes NO_INPUTS_MESSAGE, per #158: a message the user can see is
named once, so a test asserts the string the worker writes rather than a copy that can
drift. And the JVM now covers that arm, which ran before staging and before any native
code and had no business being a device test.

579 -> 584 JVM tests, 0 failures.
ConcatWorker: 14 -> 5 missed lines, 2 -> 0 missed branches.
Line 2173/2348 -> 2183/2352; branch 1091/1342 unchanged in the numerator.

The line denominator moved 2348 -> 2352: that is the ConcatJoiner interface, not new
untested code. Said plainly because CLAUDE.md's coverage entry has a history of explaining
its own numbers wrongly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 22:06:52 -05:00
JMR-devandClaude Opus 5 794cef7b34 C2 (#177): cut MediaProbe's two-probe merge into a seam, and ask which probe wins
probe() runs MediaExtractor and FFprobe independently and merges the two, and every rule
in that merge is a decision nothing held. The reason is structural rather than an
oversight: RemuxTest drives the whole thing on a device against committed fixtures, but
only ever with one probe answering and the other agreeing or also failing. Nothing on any
source set can arrange for a real extractor and a real FFprobe to *disagree*, so every
elvis in the merge was taken in one direction and never the other.

The seam is `internal fun merge(Extracted?, FFprobeInfo?): InputProbe`, pulled out of
probe() whole -- probe() now reads the two probes, merges, and keeps the log. FFprobeInfo
becomes internal alongside it; Extracted already was, with a KDoc giving this exact reason,
and FFprobeInfo simply never got the same treatment. Half a signature being private is
what made the function unnameable from a test.

Eleven tests, and the mutations that hold them:

  image beats a real video codec      demote the isImage arm below the video arm
  the extractor wins on codecs        flip the elvis to FFprobe-first
  duration is the larger reading      replace maxOf with extractor-first
  dimensions prefer the extractor     flip the width elvis
  no recognised stream is unreadable  narrow the guard to `extracted == null && info == null`

All five red, then restored. One mutation I tried first was *semantically equivalent* --
moving the image arm above the both-null arm changes nothing for any reachable input -- so
it stayed green and is recorded here rather than counted: a green mutation is only evidence
when the mutation is a real change.

The last row is the arm the ticket was filed for: parsed, and carrying no stream either
probe recognised, which is what a container holding only subtitles looks like. Its input
was already being constructed elsewhere in the suite -- MediaProbeTrackWalkTest calls
extractedFrom(emptyList()) and gets exactly it -- and had never been handed to the merge.

568 -> 579 JVM tests, 0 failures.
MediaProbe: 35 -> 24 missed lines, 72 -> 40 missed branches.
Line 2103/2348 -> 2173/2348; branch 1029/1340 -> 1091/1342.

The branch denominator moved by two, and it is the seam that moved it -- worth stating
separately from the numerator, because CLAUDE.md's coverage entry has a documented history
of explaining its own numbers wrongly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:59:00 -05:00
JMR-devandClaude Opus 5 eded47d666 B1 (#173): tell the rail from the bottom bar, and the Convert tab from the Join tab
Two assertion gaps, not coverage gaps, which is why they lasted. AppRootRestorationTest
already drives AppRoot at Compact and Expanded, so JaCoCo is green on useRail -- but it
asserts only that the selected tab survives recreation, through a stub `content`
composable. Nothing anywhere queried for a rail or a bar, and nothing composed the real
screens. Measured before this file existed:

  - transposing the NavigationRail and NavigationBar bodies passed the entire suite
  - transposing Content's two arms passed it too

A tablet showing phone chrome, or the Convert tab opening the Join screen, and 546 tests
with nothing to say about either. AppRoot's own KDoc is why that matters more than it
looks: from targetSdk 37 the app is resized and rotated whether or not it is ready, so the
width class is not a preference.

WindowWidthSizeClass.Medium appears in no test in either source set today. useRail is
`!= Compact`, so Medium takes the rail; narrowing it to `== Expanded` is one character and
breaks every tablet and unfolded foldable. That mutation is red now, and it is red only
because of the Medium test -- the Compact and Expanded ones both survive it.

Two things this needed:

**createAndroidComposeRule rather than createComposeRule.** Rendering AppRoot with its
default content reaches ConverterScreen's `viewModel = viewModel()`, which needs a
ViewModelStoreOwner. It works because both ViewModels are
`@JvmOverloads constructor(app: Application, ...)` so AndroidViewModelFactory can build
them, and because ui-test-manifest's debugImplementation entry already puts a
ComponentActivity in the merged manifest the unit tests build against -- which
app/build.gradle.kts says in terms. Checked with a throwaway spike before the ticket was
filed, rather than discovered here.

**Two tags, applied inside main.** The only production change: TestTags.Shell, set on the
rail and the bar. There is no other way to tell the two apart -- both render the same two
destinations with the same labels and the same selection state, so any assertion writable
without them is satisfied by either layout. In TestTags and applied by the shell rather
than handed down by the test, for the reason that file's KDoc gives: a tag the test
supplies proves only that the test set it. TagTableUniquenessTest covers the new group.

564 -> 568 JVM tests, 0 failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:54:50 -05:00
JMR-devandClaude Opus 5 b41341a1cb B3 (#175): pin the AAC arm every ordinary conversion takes
audioArgs has six arms. Five are named codecs with tests; AAC arrives through the `else`,
so nothing named it -- neither "aac" nor "192k" appeared anywhere in
FFmpegCommandBuilderTest. It is the audio MP4 and M4A get, which is to say the audio the
picker offers first and most conversions produce.

Both halves are asserted, and the bitrate is the half worth arguing for: an -b:a that
quietly changed would fail nothing, look wrong in no command line, and surface only as
files that sound different from the ones the app produced last month. Both mutations
confirmed red -- 192k -> 128k and aac -> libfdk_aac.

Asserted through MP4_H264 and M4A_AAC rather than one of them, so an AAC arm added above
the `else` later has to keep answering the same way for both.

**Deliberately not added here: an ENCODABLE_AUDIO-vs-audioArgs agreement test**, the
obvious companion to VideoCodecMimeAgreementTest. It would freeze the answer to F1, which
is open: ContainerCapabilities.kt:84 says "nothing here emits a Vorbis encoder" and
FFmpegCommandBuilder.kt:188 does. docs/coverage-read-findings.md says in terms that the
tempting fix there locks in the wrong answer and that the decision comes first. This is
the AAC arm only.

563 -> 564 JVM tests, 0 failures. No production code changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:50:31 -05:00
JMR-devandClaude Opus 5 0f842243b5 B2 (#174): read what the progress notification actually says
An assertion gap rather than a coverage one, which is the reason it survived. JaCoCo is
green on build()'s `if (indeterminate)` because ProgressNotificationTest drives it through
a real worker -- but that test reads the notification id and EXTRA_PROGRESS and nothing
else. Nothing had ever read the text. Swapping the two branches passed the whole suite;
so did replacing the caller's title with a constant. Both are red now.

What it costs to get wrong is small and permanent: a conversion four minutes in still
saying "Preparing", or one that has not started reporting yet claiming 0%. Neither is a
crash, and nothing else here would have found it.

Nothing in the suite had constructed ConversionNotifications directly, and the reason
turned out to be mechanical rather than an oversight: build() reaches
WorkManager.getInstance for the Cancel action's PendingIntent, so the notification cannot
be built without one. installTestWorkManager in setUp is the whole fixture, and the KDoc
records the coupling so the next person does not rediscover it.

areEnabled() in the same file is deliberately still untested. It has no caller anywhere in
app/src/main, so a test would assert that a function nobody calls returns what the platform
told it -- and would imply the app handles the disabled-notification case, which it does
not. That is F5 in docs/coverage-read-findings.md, and it asks for a decision rather than a
test.

561 -> 563 JVM tests, 0 failures. No production code changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:48:36 -05:00
JMR-devandClaude Opus 5 2fbc957119 A5 (#171): fire the muxer guard that repairs "MP4 for everything", which had never fired
Media3Muxers' KDoc names the defect this guards -- "the router claimed five containers
while the engine silently wrote MP4 for all of them" -- and the repair itself was
untested: Media3Engine$buildTransformer$3, the requireNotNull message lambda, was four
lines and four branches at 0%. Nothing had ever driven a plan whose container Media3
cannot mux, and factoryFor answers null for fourteen of them.

Weakening it does not crash. The wrong output is a playable file with the wrong
container, which is why a test rather than a bug report is what would catch it.

Same harness and the same two disciplines as Media3EngineEmptyCompositionTest, which is
the sibling this joins: assert the plan really is the one the test needs before driving
the engine, and rule out CancellationException so an unresumed continuation cannot read
as a pass. Three premises are asserted here rather than assumed -- that the plan is still
WebM by the time the engine sees it, that Media3 really has no muxer for WebM, and that
neither track was dropped, since the empty-composition refusal fires earlier and is a
different test's subject.

The assertion is on the exception type *and* its message, and the ticket predicted why:
replacing requireNotNull with `?: DefaultMuxer.Factory()` does not make the export
succeed, it lets it run on and fail some other way. Measured -- that mutation fails the
type assertion, so the guard is genuinely what this test is holding, and the message
assertion stands behind it.

560 -> 561 JVM tests, 0 failures. No production code changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:45:43 -05:00
14 changed files with 904 additions and 20 deletions
+63 -3
View File
@@ -130,9 +130,9 @@ install for code that can never run — and on API 37 the full APK does not fit
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
decision layer, where one branch is one documented user-visible outcome and the metric counts
answers rather than complexity. Every other rule still applies there.
- **Coverage is reported, not gated** — **88.9% of lines (2087/2348), 75.4% of branches
(1011/1340)**, measured 2026-08-29 with `./gradlew :app:jacocoTestReport`, against 546 JVM tests
in 76 classes.
- **Coverage is reported, not gated** — **92.8% of lines (2183/2352), 81.3% of branches
(1091/1342)**, measured 2026-09-02 with `./gradlew :app:jacocoTestReport`, against 584 JVM tests
in 87 classes.
**Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
@@ -174,6 +174,66 @@ install for code that can never run — and on API 37 the full APK does not fit
denominator shrank is not the same claim as one that rises because more branches are tested, and
this entry has a history of explaining its own numbers wrongly.
Then wave 3 (#167-#178) on 2026-09-02 — 88.9% -> 92.8% line, 75.4% -> **81.3%** branch, 546 ->
584 tests, in ten PRs from #179 to #188.
**Its shape is different from the two before it, and the difference is the thing to carry
forward.** Waves 1 and 2 were finding uncovered code. By wave 3 there was not much of that left,
so the gaps were sorted into two kinds before any test was written:
- **coverage gaps** — the line never executes. Filtered to sites where JaCoCo reports `mi > 0`, a
concrete instruction no test runs, which is what separates a real gap from a partial branch on
a compound condition. That filter cut the candidate list roughly in half and was right to.
- **assertion gaps** — JaCoCo is green and nothing checks the answer. `MainActivity`'s rail and
bottom bar were both *executed* by `AppRootRestorationTest` and **transposing them passed the
entire suite**; so did swapping the two progress-notification strings, and swapping `Content`'s
two destinations. No coverage number would ever have found any of the three.
So **every ticket named the mutation that had to go red, and that was its acceptance criterion
rather than a coverage delta**. It caught **two vacuous tests written in the same session**,
before either shipped:
- a `firstContainerHolding` test asserting a refusal still offered *something*. True, and
useless: the source container is a candidate in its own right, so the list stays non-empty
whatever the fallback does. What it actually buys is the codec the user asked for.
- a staged-delete test scanning for a `"join-"` prefix `StagingNames.forJob` does not produce —
it names files `<jobId>.<ext>`, so the assertion was true of everything.
It also corrected a *third* test that was not vacuous: `probeForConcat`'s KDoc claimed to drive
the `catch` arm, and rethrowing from that catch left it green. That is how the arm turned out to
be unreachable — see the next paragraph. A passing test with a wrong explanation is its own
failure mode.
**A green mutation is only evidence when the mutation is a real change**, which is the mirror
trap: one `classify` mutation stayed green because reordering two arms was semantically
equivalent for every reachable input. A bad mutation and a weak test look identical in the output.
Three things came back **not as the ticket described them**, which is a result rather than a
shortfall:
- `ContainerCapabilities:282`'s `exclude` filter **cannot drop anything**. `repair` always
changes a codec on the shared container — a codec it left alone is one `validate` would not
have refused — and the one non-default `exclude` carries `COPY` while every candidate carries
`NONE`. F4-shaped.
- `probeForConcat`'s catch arm is **unreachable on this runtime**. Robolectric's `MediaExtractor`
never throws from `setDataSource`, measured across an unregistered `content://` authority, a
missing `file://`, a file of garbage bytes and an `http://` URL — all four returned with
`trackCount = 0`. It stays device-only.
- `JobSnapshots:31`'s missed arm was **not** the `!isFile` one the ticket named — that is already
covered by the `reclaimed` fixture. It was `path == null`: a job carrying no output path at
all. Read the report, not the ticket, when the two disagree.
One item was **included against** the F4 rule rather than exempted by it, and the distinction is
worth having written down since both live in the same function: `ContainerCapabilities:94`
(`accepts(container, VideoCodec.NONE, mode)`) is dead in production today — every caller guards
`NONE` first — and was tested anyway, because its audio twin at `:101` has had a test since #136
and the asymmetry was the argument. The `COPY -> error(...)` arms beside it stay exempt, because
a second line of defence that can be provoked is not one.
Denominators moved here too, in both directions and for two different reasons: 1340 -> 1342
branches from `MediaProbe.merge`, 2348 -> 2352 lines from the `ConcatJoiner` interface. Neither
is new untested code.
And **re-measure before quoting**: this entry was once written quoting 81.4%, measured four hours
earlier, and was already three points stale by the time it was ready to merge.
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
@@ -24,9 +24,11 @@ import androidx.compose.runtime.saveable.Saver
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.testTag
import androidx.media3.common.util.UnstableApi
import org.libremediaconverter.convert.ConverterScreen
import org.libremediaconverter.join.JoinScreen
import org.libremediaconverter.ui.TestTags
import org.libremediaconverter.ui.theme.LibreMediaConverterTheme
/**
@@ -114,7 +116,7 @@ internal fun AppRoot(
if (useRail) {
Row(modifier = Modifier.fillMaxSize()) {
NavigationRail {
NavigationRail(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_RAIL)) {
Destination.entries.forEach { item ->
NavigationRailItem(
selected = destination == item,
@@ -132,7 +134,7 @@ internal fun AppRoot(
Scaffold(
modifier = Modifier.fillMaxSize(),
bottomBar = {
NavigationBar {
NavigationBar(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_BAR)) {
Destination.entries.forEach { item ->
NavigationBarItem(
selected = destination == item,
@@ -50,19 +50,42 @@ object MediaProbe {
)
fun probe(context: Context, uri: Uri): InputProbe {
val extracted = probeWithExtractor(context, uri)
val info = probeWithFFprobe(context, uri)
val merged = merge(probeWithExtractor(context, uri), probeWithFFprobe(context, uri))
if (merged.kind == InputKind.UNPARSEABLE) {
// Not a failure: an unparseable input is a strong signal that this job belongs on
// FFmpeg. Reporting an unknown codec makes the router say so.
Log.i(TAG, "Neither MediaExtractor nor FFprobe could read $uri; routing to FFmpeg.")
}
return merged
}
/**
* What the two probes together say about one input.
*
* A pure function, and `internal` for the same reason [extractedFrom] is: the precedence rules
* below are the answer to "which probe wins", and until this was pulled out of [probe] the only
* way to ask was to have a real `MediaExtractor` and a real FFprobe **disagree**, which nothing
* on any source set can arrange. `RemuxTest` drives this on a device against committed
* fixtures, but only ever with one probe answering and the other agreeing or also failing --
* so every elvis here was taken in one direction and never the other.
*
* The rules, each of which is a decision rather than an accident:
*
* - **The extractor wins on codecs.** It is the platform's own view of what it can decode,
* which is the thing the router is about to ask about. FFprobe's name for the same track can
* differ, and the copy planner keys off these strings.
* - **FFprobe alone reports the container.** `MediaExtractor` cannot, which is why [InputProbe]
* carries a nullable one and `CopyPlanner` treats null as "container unknown".
* - **Duration is the larger of the two**, not the first non-zero. Either probe can report zero
* for a file the other times correctly, and a zero duration makes the FFmpeg progress
* percentage undefined.
*/
internal fun merge(extracted: Extracted?, info: FFprobeInfo?): InputProbe {
val videoCodec = extracted?.videoCodec ?: info?.videoCodec
val audioCodec = extracted?.audioCodec ?: info?.audioCodec
val kind = classify(extracted, info)
if (kind == InputKind.UNPARSEABLE) {
// Not a failure: an unparseable input is a strong signal that this job belongs on
// FFmpeg. Reporting an unknown codec makes the router say so.
Log.i(TAG, "Neither MediaExtractor nor FFprobe could read $uri; routing to FFmpeg.")
return UNREADABLE
}
if (kind == InputKind.UNPARSEABLE) return UNREADABLE
return InputProbe(
videoCodec = videoCodec,
@@ -83,7 +106,7 @@ object MediaProbe {
* audio file and a corrupt file indistinguishable. The source-info card cannot describe either
* honestly until they are separate, and neither can the copy planner.
*/
private fun classify(extracted: Extracted?, info: FFprobeInfo?): InputKind = when {
internal fun classify(extracted: Extracted?, info: FFprobeInfo?): InputKind = when {
info?.isImage == true -> InputKind.IMAGE
extracted == null && info == null -> InputKind.UNPARSEABLE
(extracted?.videoCodec ?: info?.videoCodec) != null -> InputKind.VIDEO
@@ -162,7 +185,11 @@ object MediaProbe {
}
}
private class FFprobeInfo(
/**
* `internal` rather than `private` for the same reason [Extracted] is, and it should have been
* from the start: [merge] cannot be named from a test while half its signature is private.
*/
internal class FFprobeInfo(
val container: Container?,
val videoCodec: String?,
val audioCodec: String?,
@@ -4,6 +4,7 @@ import android.content.Context
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import org.libremediaconverter.codec.AndroidDeviceCodecs
import org.libremediaconverter.ffmpeg.ConcatEngine
import org.libremediaconverter.ffmpeg.FFmpegEngine
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.DeviceCodecs
@@ -40,6 +41,27 @@ interface SoftwareTranscoder {
)
}
/**
* The join path. Implemented by [org.libremediaconverter.ffmpeg.ConcatEngine].
*
* Added last of the three, and the gap it closes was measured rather than guessed:
* `PerJobStagingTest`'s KDoc records that reverting `ConcatWorker` to a constant staging name left
* all 257 tests green, because nothing in the JVM suite can get past a `ConcatEngine` constructed
* in place. Everything after that line -- the failure mapping, the message fallback, the staged
* delete -- was untested on every source set.
*
* The result type stays nested in the implementation rather than being lifted here. Moving it would
* touch every call site to buy nothing: what a test needs is the ability to *not* run FFmpeg, and
* that is the method, not the type.
*/
interface ConcatJoiner {
suspend fun join(
inputs: List<Uri>,
output: File,
format: OutputFormat = OutputFormat.MP4_H264,
): ConcatEngine.Result
}
/**
* The seam that lets tests force failure paths.
*
@@ -69,6 +91,9 @@ object ConversionDependencies {
@Volatile
var software: () -> SoftwareTranscoder = { FFmpegEngine() }
@Volatile
var concat: (Context) -> ConcatJoiner = { ConcatEngine(it) }
@Volatile
var publisher: (Context) -> OutputPublisher = { OutputPublisher(it) }
@@ -103,6 +128,7 @@ object ConversionDependencies {
fun reset() {
hardware = { Media3Engine(it) }
software = { FFmpegEngine() }
concat = { ConcatEngine(it) }
publisher = { OutputPublisher(it) }
deviceCodecs = { AndroidDeviceCodecs.get() }
probe = { context, uri -> MediaProbe.probe(context, uri) }
@@ -7,6 +7,7 @@ import com.arthenica.ffmpegkit.FFmpegKit
import com.arthenica.ffmpegkit.FFmpegKitConfig
import com.arthenica.ffmpegkit.ReturnCode
import kotlinx.coroutines.suspendCancellableCoroutine
import org.libremediaconverter.convert.ConcatJoiner
import org.libremediaconverter.convert.MediaProbe
import org.libremediaconverter.convert.StagingNames
import org.libremediaconverter.model.ConcatPlanner
@@ -24,11 +25,11 @@ import kotlin.coroutines.resumeWithException
* reliably fail when they differ — it can emit a file whose later segments are
* garbled. See [ConcatPlanner].
*/
class ConcatEngine(private val context: Context) {
class ConcatEngine(private val context: Context) : ConcatJoiner {
data class Result(val strategy: ConcatStrategy, val output: File)
suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat = OutputFormat.MP4_H264): Result {
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): Result {
require(inputs.size >= 2) { "Joining needs at least two files." }
val paths = inputs.map { uri ->
@@ -56,6 +56,20 @@ object TestTags {
*/
const val RETRY_SAVE: String = "action.retrySave"
/**
* The adaptive shell around both screens -- `AppRoot`'s two layouts.
*
* Named because there is no other way to tell them apart from a test. Both render the same two
* destinations with the same labels and the same selection state, so every assertion that could
* be written without these tags is satisfied by either layout, and transposing the two bodies
* passed the whole suite. Exactly one of the two exists at a time, which is what makes
* `assertExists` / `assertDoesNotExist` on this pair a statement about the width class.
*/
object Shell {
const val NAVIGATION_RAIL: String = "shell.navigationRail"
const val NAVIGATION_BAR: String = "shell.navigationBar"
}
/** `ConverterScreen`. */
object Converter {
const val CHOOSE_FILE: String = "converter.chooseFile"
@@ -15,7 +15,6 @@ import kotlinx.coroutines.CancellationException
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.InputQuery
import org.libremediaconverter.convert.StagingNames
import org.libremediaconverter.ffmpeg.ConcatEngine
import org.libremediaconverter.model.OutputFormat
/**
@@ -37,7 +36,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
override suspend fun doWork(): Result {
val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse)
?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
?: return Result.failure(workDataOf(KEY_ERROR to NO_INPUTS_MESSAGE))
if (uris.size < 2) {
return Result.failure(workDataOf(KEY_ERROR to TOO_FEW_INPUTS_MESSAGE))
}
@@ -77,7 +76,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
),
)
val result = ConcatEngine(applicationContext).join(uris, staged, format)
val result = ConversionDependencies.concat(applicationContext).join(uris, staged, format)
Result.success(
workDataOf(
KEY_OUTPUT_PATH to staged.absolutePath,
@@ -148,6 +147,16 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
* Here rather than in the ViewModel because the rule is the worker's: `request(...)` takes
* a `List<Uri>` and checks nothing about its length, so this is the guard that always runs.
*/
/**
* A job carrying no input array at all -- a downgrade, or a queue entry from a build that
* spelled the key differently.
*
* A constant rather than the literal it was, for the convention #158 established: a message
* the user can see is named once, so a test asserts the same string the worker writes
* rather than a copy of it that can drift.
*/
const val NO_INPUTS_MESSAGE: String = "No input files."
const val TOO_FEW_INPUTS_MESSAGE: String = "Pick at least two files to join."
/**
@@ -0,0 +1,129 @@
package org.libremediaconverter
import androidx.activity.ComponentActivity
import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import org.junit.After
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
/**
* Which navigation affordance the shell actually renders, and which screen it actually shows.
*
* Assertion gaps rather than coverage gaps, both of them, and that is why they lasted.
* `AppRootRestorationTest` already drives `AppRoot` at `Compact` and `Expanded`, so JaCoCo is green
* on `useRail` -- but it asserts only that the *selected tab* survives recreation, through a stub
* `content` composable. Nothing anywhere queried for a rail or a bar, and nothing rendered the real
* screens. Two consequences, both measured before this file existed:
*
* - **Transposing the `NavigationRail` and `NavigationBar` bodies passed the entire suite.**
* - **Transposing `Content`'s two arms passed it too** -- a tablet showing the phone chrome, or the
* Convert tab opening the Join screen, and 546 tests with nothing to say about either.
*
* `AppRoot`'s own KDoc is why this matters more than it looks: from targetSdk 37 the app is resized
* and rotated whether or not it is ready, so the width class is not a preference, it is whatever
* the system hands over.
*
* ## Two things this needed that the rest of the suite does not
*
* **`createAndroidComposeRule`, not `createComposeRule`.** Rendering `AppRoot` with its *default*
* content reaches `ConverterScreen`'s `viewModel = viewModel()`, which needs a
* `ViewModelStoreOwner`; the plain rule supplies none. It works because both ViewModels are
* `@JvmOverloads constructor(app: Application, …)`, so `AndroidViewModelFactory` can build them,
* and because `app/build.gradle.kts` already puts `ui-test-manifest`'s `ComponentActivity` in the
* merged manifest the unit tests build against -- which that file says in terms.
*
* **Tags on the two bars.** They are in `TestTags`, applied inside `main`, for the reason that
* file's KDoc gives: a tag the test hands down proves only that the test set it.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class AdaptiveShellTest {
@get:Rule
val composeRule = createAndroidComposeRule<ComponentActivity>()
@Before
fun setUp() {
val app = RuntimeEnvironment.getApplication()
installTestWorkManager(app, Data.EMPTY)
// The real screens are composed here, so their ViewModels are real too. Neither test is
// about probing or publishing; left alone they would reach the FFprobe loader and this
// machine's codec list, and decide things no assertion mentions.
ConversionDependencies.probe = { _, _ -> InputProbe() }
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `a phone gets the bottom bar and a tablet gets the rail`() {
setShell(WindowWidthSizeClass.Compact)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertDoesNotExist()
}
@Test
fun `an expanded window gets the rail`() {
setShell(WindowWidthSizeClass.Expanded)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
}
/**
* The width class no test had ever passed.
*
* `useRail` is `!= Compact`, so Medium takes the rail with Expanded. Narrowing it to
* `== Expanded` is a one-character change that breaks every tablet and unfolded foldable and
* nothing else -- and until this test, nothing in either source set used `Medium` at all.
*/
@Test
fun `a medium window is a rail window, not a phone`() {
setShell(WindowWidthSizeClass.Medium)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
}
/**
* The mapping every other test stubs out: which screen each destination actually opens.
*
* Matched on each screen's own "choose a file" affordance rather than on a title, because those
* tags are applied by the screens themselves -- so this fails if the destinations are
* transposed, and it fails for the right reason.
*/
@Test
fun `Convert opens the converter and Join opens the join screen`() {
setShell(WindowWidthSizeClass.Compact)
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertExists()
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertDoesNotExist()
composeRule.onNodeWithText(Destination.JOIN.label).performClick()
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertExists()
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertDoesNotExist()
}
/** [AppRoot] with its real content, which is the half nothing else composes. */
private fun setShell(width: WindowWidthSizeClass) {
composeRule.setContent { AppRoot(width) }
}
}
@@ -0,0 +1,99 @@
package org.libremediaconverter.convert
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.AudioPlan
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.CopyPlanner
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.model.VideoPlan
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.concurrent.CancellationException
/**
* A job that reached Media3 with a container Media3 cannot mux.
*
* [Media3Muxers]' own KDoc names the defect this guards: *"the router claimed five containers while
* the engine silently wrote MP4 for all of them."* `factoryFor` answers null for fourteen of the
* app's containers, and `buildTransformer` turns that null into a failed job rather than letting
* `Transformer` fall back to its default muxer.
*
* The guard had never fired. `Media3Engine$buildTransformer$3` -- the `requireNotNull` message
* lambda -- was four lines and four branches at 0%, which is to say the entire repair for a defect
* the codebase went to the trouble of writing down was untested. Weakening it would restore that
* bug silently, because the wrong output is a *playable file with the wrong container*, not a crash.
*
* Same harness and same two disciplines as [Media3EngineEmptyCompositionTest]: assert the plan
* really is the one the test needs before driving the engine, and rule out
* `CancellationException` so an unresumed continuation cannot read as a pass.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class Media3MuxerGuardTest {
@Test
fun `a container Media3 cannot mux fails the job rather than silently writing MP4`() {
val context = RuntimeEnvironment.getApplication()
val engine = Media3Engine(context)
val request = ConversionRequest(
spec = OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.OPUS),
probe = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MP4),
)
// The premise, asserted rather than assumed -- three separate ways this test could pass
// over a path it never entered.
val plan = CopyPlanner.plan(request.spec, request.probe)
assertEquals("the plan has to still be WebM by the time the engine sees it", Container.WEBM, plan.container)
assertNull("...and Media3 really has no muxer for it", Media3Muxers.factoryFor(plan.container))
// Not the empty-composition refusal, which fires earlier and is a different test's subject.
assertNotEquals(VideoPlan.Drop, plan.video)
assertNotEquals(AudioPlan.Drop, plan.audio)
val failure = try {
runCatching {
runBlocking {
withTimeout(TIMEOUT_MS) {
engine.transcode(Uri.parse("file:///dev/null"), File(context.cacheDir, "guard.webm"), request) {
}
}
}
}.exceptionOrNull()
} finally {
engine.close()
}
assertFalse(
"the continuation was never resumed -- the refusal escaped instead of failing the job: $failure",
failure is CancellationException,
)
// Type *and* message, and the message half is the load-bearing one. Replacing the
// requireNotNull with a fallback factory does not make the export succeed here: it lets it
// run on and fail some other way, which a bare type assertion would happily accept.
assertTrue("expected the muxer guard to refuse the job, got $failure", failure is IllegalArgumentException)
assertTrue(
"the refusal has to name the container it could not mux, got: ${failure?.message}",
failure?.message.orEmpty().contains("cannot mux") &&
failure?.message.orEmpty().contains(Container.WEBM.name),
)
}
private companion object {
/** Nothing is decoded or muxed on this path -- the guard refuses before any of that. */
const val TIMEOUT_MS = 10_000L
}
}
@@ -0,0 +1,166 @@
package org.libremediaconverter.convert
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.InputKind
import org.libremediaconverter.model.InputProbe
/**
* Which of the two probes wins, when they disagree.
*
* [MediaProbe.probe] runs `MediaExtractor` and FFprobe independently and then merges the two, and
* every rule in that merge is a decision. None of them had a test, for a reason that is structural
* rather than an oversight: `RemuxTest` drives the whole thing on a device against committed
* fixtures, but only ever with **one probe answering and the other agreeing or also failing**.
* Nothing on any source set can arrange for a real extractor and a real FFprobe to disagree, so
* every elvis in the merge was taken in one direction and never the other.
*
* Cutting `merge` out of `probe` is what makes the question askable. Both halves of its signature
* had to become `internal` for that -- `Extracted` already was, with a KDoc giving this exact
* reason; `FFprobeInfo` simply never got the same treatment.
*/
class MediaProbeMergeTest {
/**
* The rule with the loudest failure mode, and `isImageFormat`'s own KDoc names it: a false
* positive here "makes the source-info card describe a video as an image". So the image verdict
* has to beat a real video codec from the extractor, and the ordering that makes it do so is
* the first arm of `classify` rather than anything a reader would infer from the fields.
*/
@Test
fun `an image verdict from FFprobe beats a video codec from the extractor`() {
val merged = MediaProbe.merge(
extracted = extracted(video = "h264"),
info = info(video = "mjpeg", isImage = true),
)
assertEquals(InputKind.IMAGE, merged.kind)
}
@Test
fun `the extractor wins on codecs, because it is the view the router will act on`() {
val merged = MediaProbe.merge(
extracted = extracted(video = "h264", audio = "aac"),
info = info(video = "hevc", audio = "mp3"),
)
assertEquals("h264", merged.videoCodec)
assertEquals("aac", merged.audioCodec)
}
@Test
fun `FFprobe answers for a file the extractor could not open`() {
val merged = MediaProbe.merge(extracted = null, info = info(video = "vp9", audio = "opus"))
assertEquals("vp9", merged.videoCodec)
assertEquals("opus", merged.audioCodec)
assertEquals(InputKind.VIDEO, merged.kind)
}
@Test
fun `the extractor answers for a file FFprobe could not read`() {
val merged = MediaProbe.merge(extracted = extracted(video = "h264", audio = "aac"), info = null)
assertEquals("h264", merged.videoCodec)
assertEquals("aac", merged.audioCodec)
assertNull("only FFprobe can name the container, so it stays unknown here", merged.container)
}
/**
* The larger of the two, not the first non-zero.
*
* Either probe can report zero for a file the other times correctly, and a zero duration makes
* the FFmpeg progress percentage undefined -- `FFmpegEngine` divides by it. Both orderings are
* asserted because "take the extractor's" and "take the larger" agree in one direction and not
* the other, and only one of them is the rule.
*/
@Test
fun `duration is the longer of the two readings, whichever probe supplied it`() {
assertEquals(
5_000L,
MediaProbe.merge(extracted(duration = 0L), info(duration = 5_000L)).durationMs,
)
assertEquals(
5_000L,
MediaProbe.merge(extracted(duration = 5_000L), info(duration = 0L)).durationMs,
)
}
@Test
fun `dimensions come from the extractor, and from FFprobe only when it has none`() {
assertEquals(1920, MediaProbe.merge(extracted(width = 1920), info(width = 640)).width)
assertEquals(640, MediaProbe.merge(extracted = null, info = info(width = 640)).width)
assertEquals(0, MediaProbe.merge(extracted(width = 0), info(width = 0)).width)
}
@Test
fun `the container comes from FFprobe, which is the only probe that can name one`() {
val merged = MediaProbe.merge(extracted(video = "h264"), info(container = Container.MKV))
assertEquals(Container.MKV, merged.container)
}
@Test
fun `a file with audio and no video is audio-only, not unparseable`() {
val merged = MediaProbe.merge(extracted(video = null, audio = "mp3"), info = null)
assertEquals(InputKind.AUDIO_ONLY, merged.kind)
assertFalse(merged.hasVideo)
}
@Test
fun `a file neither probe could open is the one unreadable answer`() {
val merged = MediaProbe.merge(extracted = null, info = null)
assertEquals(MediaProbe.UNREADABLE, merged)
assertEquals(InputProbe.UNPARSEABLE, merged.videoCodec)
}
/**
* The arm the ticket was filed for: parsed, and carrying no stream either probe recognised.
*
* Distinct from "neither probe could open it" -- here the extractor opened the file happily and
* found nothing convertible, which is what a container holding only subtitles looks like. It
* has to reach the same [MediaProbe.UNREADABLE] answer, because the router keys off that and
* there is nothing here for Media3 to do either way.
*
* Its input was already being built elsewhere in the suite -- `MediaProbeTrackWalkTest` calls
* `extractedFrom(emptyList())` and gets exactly this -- and had simply never been handed to the
* merge.
*/
@Test
fun `a file that parsed but carries no recognised stream is unreadable too`() {
val merged = MediaProbe.merge(extracted = MediaProbe.extractedFrom(emptyList()), info = null)
assertEquals(InputKind.UNPARSEABLE, merged.kind)
assertEquals(MediaProbe.UNREADABLE, merged)
}
@Test
fun `hasVideo follows the codec that survived the merge, not either probe alone`() {
assertTrue(MediaProbe.merge(extracted(video = null), info(video = "vp9")).hasVideo)
assertFalse(MediaProbe.merge(extracted(video = null, audio = "aac"), info(video = null)).hasVideo)
}
private fun extracted(
video: String? = "h264",
audio: String? = "aac",
duration: Long = 1_000L,
width: Int = 1280,
height: Int = 720,
) = MediaProbe.Extracted(video, audio, duration, width, height)
private fun info(
container: Container? = null,
video: String? = "h264",
audio: String? = "aac",
duration: Long = 1_000L,
width: Int = 1280,
height: Int = 720,
isImage: Boolean = false,
) = MediaProbe.FFprobeInfo(container, video, audio, duration, width, height, isImage)
}
@@ -166,6 +166,30 @@ class FFmpegCommandBuilderTest {
assertPair(cmd(OutputFormat.OPUS), "-c:a", "libopus")
}
/**
* The arm most conversions actually take, and the only one in `audioArgs` with no test.
*
* `flac wav and opus select the right encoders` above covers the three named arms; MP3 has its
* own. AAC arrives through the `else`, so nothing named it and nothing pinned either half of
* what it emits -- neither `aac` nor `192k` appeared anywhere in this file. Both are shipped
* defaults: MP4 and M4A are the formats the picker offers first, so this is the audio
* every ordinary conversion gets.
*
* The bitrate is asserted as well as the encoder because it is the half a refactor is likelier
* to lose. An `-b:a` that quietly changed would not fail anything, would not look wrong in a
* command line, and would show up only as files that sound different from the ones the app
* produced last month.
*/
@Test
fun `aac is the default encoder, at the bitrate the app ships`() {
assertPair(cmd(OutputFormat.MP4_H264), "-c:a", "aac")
assertPair(cmd(OutputFormat.MP4_H264), "-b:a", "192k")
// Through the `else` rather than through a named arm, so an AAC branch added above it later
// has to keep answering the same way.
assertPair(cmd(OutputFormat.M4A_AAC), "-c:a", "aac")
assertPair(cmd(OutputFormat.M4A_AAC), "-b:a", "192k")
}
@Test
fun `audio only formats never carry a video encoder`() {
listOf(OutputFormat.MP3, OutputFormat.FLAC, OutputFormat.WAV, OutputFormat.OPUS)
@@ -28,6 +28,7 @@ class TagTableUniquenessTest {
fun `every tag constant has its own value`() {
val tags = tagsIn(
TestTags::class.java,
TestTags.Shell::class.java,
TestTags.Converter::class.java,
TestTags.Join::class.java,
)
@@ -0,0 +1,237 @@
package org.libremediaconverter.work
import android.app.Application
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.ListenableWorker
import androidx.work.testing.TestListenableWorkerBuilder
import androidx.work.workDataOf
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConcatJoiner
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.StagingNames
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.ffmpeg.ConcatEngine
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.model.OutputFormat
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.UUID
/**
* What a join does when the engine fails partway.
*
* Everything past `ConcatWorker`'s `setForeground` was untested on **every** source set, and the
* repo had already measured the cost: `PerJobStagingTest`'s KDoc records that reverting
* `ConcatWorker` to a constant staging name left all 257 tests green, because nothing in the JVM
* suite can get past a `ConcatEngine` constructed in place. `RefusedJobTest` says the same from the
* other side -- "the next thing past the count guard is `ConcatEngine`, which is native".
* `ConcatEngineTest` on a device tests the engine directly, bypassing the worker, and
* `ConcatWorkerTest` covers only the too-few-inputs guard and the happy path.
*
* `ConversionDependencies.concat` is the seam that closes it, added here to sit beside the
* `.hardware` and `.software` that `ConversionWorker` has had all along -- the asymmetry between the
* two workers was the whole reason one of them had a tested failure path and the other did not.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ConcatFailureTest {
private lateinit var app: Application
private lateinit var publisher: AlwaysRoomPublisher
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
publisher = AlwaysRoomPublisher(app)
ConversionDependencies.publisher = { publisher }
installTestWorkManager(app, Data.EMPTY)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `a join whose engine fails reports the engine's own reason`() {
ConversionDependencies.concat = { FailingJoiner { error(DEMUXER_MESSAGE) } }
val result = runBlocking { joinWorker().doWork() }
assertEquals(
ListenableWorker.Result.failure(workDataOf(ConcatWorker.KEY_ERROR to DEMUXER_MESSAGE)),
result,
)
}
/**
* A failure carrying no message at all, which Kotlin and Java both allow and FFmpegKit's
* wrappers can produce.
*
* Without the fallback the user is shown an empty error, and `JoinViewModel` cannot tell that
* from a job that reported nothing -- the two would be one blank screen with different causes.
*/
@Test
fun `a failure with no message of its own still says something`() {
ConversionDependencies.concat = { FailingJoiner { throw RuntimeException() } }
val result = runBlocking { joinWorker().doWork() }
assertEquals(
ListenableWorker.Result.failure(
workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.GENERIC_FAILURE_MESSAGE),
),
result,
)
}
/**
* The staged file is deleted on the way out.
*
* The joiner writes before it fails, exactly as `PartialThenFailingTranscoder` does on the
* conversion side: a stub that only threw would let a missing `staged.delete()` pass. What it
* costs to lose is a full-size partial per failed join, sitting in cache until the sweep is old
* enough to be sure nobody is coming back for it.
*
* Asserted against the file the joiner was actually handed rather than by scanning the staging
* directory for a name. The first draft did scan, for a `"join-"` prefix that
* `StagingNames.forJob` does not produce -- it names files `<jobId>.<ext>` -- so the assertion
* was trivially true and the mutation walked straight through it.
*/
@Test
fun `a failed join leaves nothing behind in staging`() {
val joiner = FailingJoiner { error(DEMUXER_MESSAGE) }
ConversionDependencies.concat = { joiner }
runBlocking { joinWorker().doWork() }
val staged = requireNotNull(joiner.lastOutput) { "the joiner never ran, so this proves nothing" }
assertEquals(
"the fixture has to write before it fails, or the delete is unobservable",
PARTIAL_BYTES,
joiner.bytesWritten,
)
assertFalse("a failed join must not leave its partial behind: $staged", staged.exists())
}
/**
* The success path, and the staging name #159's fixture and `PerJobStagingTest` both care about.
*
* Worth its place rather than a happy-path formality: `PerJobStagingTest`'s KDoc records that
* **reverting `ConcatWorker` to a constant staging name left all 257 tests green**, because
* nothing could reach the line that names the file. This is the test that was missing when that
* was written -- the join's output `Data` had never been read by anything on the JVM.
*
* The staged path is asserted to carry the job id, not a constant: two joins of the same format
* sharing one name is the defect, and `ConcatEngine`'s list file collided harder still.
*/
@Test
fun `a join that works reports its own staged file, strategy and name`() {
val joiner = SucceedingJoiner()
ConversionDependencies.concat = { joiner }
val result = runBlocking { joinWorker().doWork() }
assertTrue("got $result", result is ListenableWorker.Result.Success)
val data = (result as ListenableWorker.Result.Success).outputData
assertEquals(
"the staged file has to be this job's, not a name every join shares",
File(publisherStagingDir(), StagingNames.forJob(JOB_ID, OutputFormat.MP4_H264.extension)).absolutePath,
data.getString(ConcatWorker.KEY_OUTPUT_PATH),
)
assertEquals(ConcatStrategy.STREAM_COPY.name, data.getString(ConcatWorker.KEY_STRATEGY))
assertEquals(OutputFormat.MP4_H264.mimeType, data.getString(ConcatWorker.KEY_MIME_TYPE))
assertTrue(
"the save dialog needs a name with the right extension, got ${data.getString(
ConcatWorker.KEY_SUGGESTED_NAME,
)}",
data.getString(ConcatWorker.KEY_SUGGESTED_NAME).orEmpty().endsWith(".${OutputFormat.MP4_H264.extension}"),
)
}
/**
* The arm beside the count guard: no URI array at all.
*
* Covered today only by `UnopenableUriTest` on a device, although it runs before staging and
* before any native code. It is the exact sibling of `RefusedJobTest`'s ConversionWorker twin,
* and it belongs on the JVM with it -- a device test for a branch that needs no device is a
* slower test that reports later.
*/
@Test
fun `a join with no input array at all is refused with a message`() {
val result = runBlocking {
TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name),
runAttemptCount = 0,
).setId(JOB_ID).build().doWork()
}
assertEquals(
ListenableWorker.Result.failure(workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.NO_INPUTS_MESSAGE)),
result,
)
}
private fun publisherStagingDir(): File? = publisher.createStagingFile("probe").parentFile
private fun joinWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(
ConcatWorker.KEY_INPUT_URIS to arrayOf(FIRST.toString(), SECOND.toString()),
ConcatWorker.KEY_TOTAL_BYTES to TOTAL_BYTES,
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
),
runAttemptCount = 0,
).setId(JOB_ID).build()
private companion object {
val FIRST: Uri = Uri.parse("file:///tmp/one.mp4")
val SECOND: Uri = Uri.parse("file:///tmp/two.mp4")
const val TOTAL_BYTES = 2048L
const val DEMUXER_MESSAGE = "the demuxer rejected the input list"
const val PARTIAL_BYTES = 2048
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000b")
}
}
/** A joiner that writes something and then fails, so a missing `staged.delete()` cannot pass. */
private class FailingJoiner(private val failure: () -> Nothing) : ConcatJoiner {
/** The handle the worker created, kept so a test can ask whether it survived the failure. */
var lastOutput: File? = null
var bytesWritten = 0
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
lastOutput = output
output.writeBytes(ByteArray(PARTIAL_BYTES))
bytesWritten = PARTIAL_BYTES
failure()
}
private companion object {
const val PARTIAL_BYTES = 2048
}
}
/** The joiner that finishes, so the success path and the output `Data` can be read on the JVM. */
private class SucceedingJoiner : ConcatJoiner {
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
output.writeBytes(ByteArray(OUTPUT_BYTES))
return ConcatEngine.Result(ConcatStrategy.STREAM_COPY, output)
}
private companion object {
const val OUTPUT_BYTES = 4096
}
}
@@ -0,0 +1,89 @@
package org.libremediaconverter.work
import android.app.Notification
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.installTestWorkManager
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.util.UUID
/**
* The two things a progress notification can say, and that they are not the same thing.
*
* An assertion gap rather than a coverage one, and the distinction is the reason this file exists.
* JaCoCo is green on `build`'s `if (indeterminate)`, because `ProgressNotificationTest` drives it
* through a real worker -- but that test reads only the notification id and
* `Notification.EXTRA_PROGRESS`. **Nothing had ever read the text.** Swapping the two branches, or
* collapsing them into one string, passed the entire suite.
*
* What it costs to get wrong is small and constant: a conversion that has been running for four
* minutes still saying "Preparing", or one that has not started reporting yet claiming 0%. Neither
* is a crash, and neither would be found by anything else here -- which is exactly the kind of
* thing that survives for a long time.
*
* Nothing else in the suite constructs [ConversionNotifications] directly.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class NotificationProgressTextTest {
/**
* `build` reaches `WorkManager.getInstance` for the Cancel action's PendingIntent, so the
* notification cannot be built at all without one. That coupling is why nothing had ever
* constructed this class directly and read what it produced.
*/
@Before
fun setUp() {
installTestWorkManager(RuntimeEnvironment.getApplication(), Data.EMPTY)
}
@Test
fun `an indeterminate notification says something different from a measured one`() {
val context = RuntimeEnvironment.getApplication()
val notifications = ConversionNotifications(context)
val preparing = notifications.build(JOB_ID, TITLE, percent = 0, indeterminate = true).text()
val measured = notifications.build(JOB_ID, TITLE, percent = 42, indeterminate = false).text()
assertNotEquals(
"the two states have to read differently, or the text says nothing at all",
preparing,
measured,
)
assertTrue(
"a measured notification has to carry its percentage, got \"$measured\"",
measured.contains("42"),
)
assertTrue(
"an indeterminate one must not invent one, got \"$preparing\"",
!preparing.contains("42") && !preparing.contains("0"),
)
}
/**
* The title is the caller's, not the builder's -- it is the file the user picked, and it is what
* tells two simultaneous conversions apart in the shade.
*/
@Test
fun `the notification is titled with the file it is converting`() {
val context = RuntimeEnvironment.getApplication()
val built = ConversionNotifications(context).build(JOB_ID, TITLE, percent = 10)
assertEquals(TITLE, built.extras.getString(Notification.EXTRA_TITLE))
}
private fun Notification.text(): String = extras.getString(Notification.EXTRA_TEXT).orEmpty()
private companion object {
const val TITLE = "holiday.mp4"
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000a")
}
}