Compare commits

...
Author SHA1 Message Date
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
JMR-devandClaude Opus 5 a645442acc A2 + A3 (#168, #169): the hardware fallback, the cancellation that must not take it, and the name a job may not have
runMedia3OrFallBack was eleven lines at 0% and isCancellation had never been called by any
JVM test -- ci=0, not merely a missed branch. The seam to reach it has existed the whole
time: ConversionDependencies.hardware, which no unit test had ever set. What kept the path
cold is that every worker test uses EnginePreference.FORCE_SOFTWARE, which never enters
the function, and the probe defaults to UNPARSEABLE, which PERMISSIVE.canDecode refuses --
so even AUTO would have routed straight to FFmpeg for a reason no assertion mentioned.
Both are now stated in setUp rather than inherited.

Four behaviours, each with the mutation that proves it:

  hardware failure falls back to software    delete the fallback call
  ... on a *clean* staging file              delete staged.delete() before it
  cancellation is rethrown, not fallen back  delete `if (isCancellation(e)) throw e`
  engine.close() runs either way             empty the finally block
  the display-name fallback (#169)           change "input" to anything else

All five confirmed red, then restored.

The cancellation one is the reason this ticket was first in the group. runMedia3OrFallBack
catches Throwable, so without that re-throw a user cancelling a hardware transcode has the
app quietly start a *second* conversion in software -- the one thing cancelling is for.
ForcedFailureTest covers the failure half on a device and does not cover this half at all.

The clean-staging assertion is made where it is observable rather than by reading the
file: the software fake records whether the output existed when it was entered, so a
missing delete shows up as FFmpeg finding a half-written hardware output at the path it is
about to write.

556 -> 560 JVM tests, 0 failures. No production code changed.
ConversionWorker: 22 -> 9 missed lines, 14 -> 7 missed branches.
Line 2090/2348 -> 2103/2348; branch 1022/1340 -> 1029/1340.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:42:27 -05:00
JMR-devandClaude Opus 5 5761faced6 A4 (#170): join the two halves of an unreadable join clip, and record why the catch arm stays device-only
The ticket asked for two things. One of them is not reachable from the JVM, and saying
so is most of the value here.

**probeForConcat's catch arm cannot be provoked on this runtime.** Robolectric's
MediaExtractor never throws from setDataSource -- measured across four input shapes: an
unregistered content:// authority, a missing file://, a file of garbage bytes, and an
http:// URL. All four returned normally with trackCount = 0. So a failed read arrives as
an empty track list rather than as an exception and reaches the same
ConcatInput(null, null, 0, 0, 0) by the other road. The catch stays covered only by
ConcatEngineTest on a device. The test file says this rather than implying the arm is
handled.

**What is reachable, and was genuinely missing, is the span.** Both halves were already
covered and neither reached the other: MediaProbeTrackWalkTest pins what concatInputFrom
makes of a track list, ConcatPlannerTest's `an unknown codec is not treated as a match`
pins what the planner does with a hand-built ConcatInput(video = null). The planner's
safety rests on the probe really producing that shape, and the hand-built fixture would
go on passing if it stopped.

Measured rather than claimed: mutating concatInputFrom's initial `video` to a non-null
placeholder leaves ConcatPlannerTest green and turns this red. Dropping the planner's
video null guard turns both red -- so that half was already held, and this file does not
claim credit for it.

The coupling itself is worth writing down: ConcatPlanner guards video against a null codec
and audio not at all, and that asymmetry is correct rather than an oversight --
MediaProbe.shortName returns a non-null String, so a null audioCodec means the track is
absent and two clips with no audio really do match, while a null videoCodec means absent
*or* unreadable. The audio check is safe because the video guard fires first on a clip
nothing could read. Nothing held that.

555 -> 556 JVM tests, 0 failures. No production code changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:37:45 -05:00
JMR-devandClaude Opus 5 c2c0bfa848 A6 (#172): six one-branch outcomes nothing produced, and one that cannot be produced
Each of these is a site where JaCoCo reported mi > 0 -- a concrete instruction no test
runs -- rather than a partial branch on a compound condition, which is how the group was
filtered in the first place. Six closed, one moved to the exclusions.

  AndroidDeviceCodecs:35   the UNPARSEABLE sentinel, refused where an unknown name is not
  ContainerCapabilities:94 accepts(container, VideoCodec.NONE, mode) -- the audio twin has
                           had a test since #136; the asymmetry is the argument
  ContainerCapabilities:323 repairVideo's keep-the-requested-codec arm
  ContainerCapabilities:348 firstContainerHolding's fallback container
  OutputPublisher:230      the resolver call that throws -- the third case the KDoc names
                           and the one the shape list was missing
  JobSnapshots:31          a job that recorded no output path at all

Every one was mutated and confirmed red, then restored. Two are worth stating because
they did not go red first time or would not have:

**The firstContainerHolding test was vacuous on its first draft.** It asserted the
refusal still offered *something*, and deleting the fallback left it green: the source
container is a candidate in its own right, so the list stays non-empty and only its
contents change. Rewritten around AVI, which has no mapping for H.265, and asserting the
codec survives -- without the fallback the app silently offers H.264 instead, which is
the actual loss. This is the failure mode CLAUDE.md records from the mutation review, met
head on rather than in the abstract.

**OutputPublisher:230 needed one line.** `a destination whose size cannot be determined is
never deleted` already walked three RowShapes; QUERY_THROWS was the fourth case its own
KDoc names -- "a resolver call that throws" -- and the only one that reaches `?: false`
through runCatching rather than through a cursor answer.

ContainerCapabilities:282's `.filter { it != exclude }` is **not** closed here and is not
a gap: nothing can make it drop anything. On the shared container `repair` always changes
at least one codec, because a codec it left alone is one validate would not have refused;
every other candidate differs by container; and the single call site passing a non-default
exclude (validateVideo:186) excludes a spec carrying VideoCodec.COPY while every repaired
candidate carries NONE. F4-shaped -- recorded rather than covered, and 550 tests agree.

550 -> 555 JVM tests, 0 failures. Five new tests rather than six: OutputPublisher:230
is one line inside a test that already existed. No production code changed.
Line 2087/2348 -> 2090/2348; branch 1011/1340 -> 1022/1340.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:31:21 -05:00
JMR-devandClaude Opus 5 016030f3e4 A1 (#167): pin all three foreground-service regimes, and the boundary between two of them
`ConversionForegroundType.current()` has three arms and the JVM suite executed one.
`robolectric.properties` pins everything to `sdk=36`, and `@Config` appears nowhere in
`app/src/test`, so 3 lines and 3 of 4 branches were cold.

The instrumented test is not a substitute, and the reason is specific rather than
general. `ConversionWorkerTest.foregroundTypeMatchesTheRunningApiLevel` asserts against
whichever API the leg is, so it covers one arm per leg and never the other two -- and the
legs that would cover 33 and 34 are the ones #122 wedges. From
docs/coverage-read-findings.md, an API 33 run reported `received: 60` with
`failed: unknown`: the regime was exercised and that leg could not have said so if it had
broken. This runs all three deterministically in the same ./gradlew invocation.

Four classes, not three. 35 shares its answer with 36 and looks redundant; it is the
whole point. Relaxing `>= VANILLA_ICE_CREAM` to `>` is invisible at every level except
exactly 35 -- measured, not assumed: that mutation failed ForegroundTypeApi35Test alone,
while swapping DATA_SYNC and MEDIA_PROCESSING failed 34, 35 and 36. Without the 35 class
the first mutation survives the suite.

546 -> 550 JVM tests, 0 failures. No production code changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:19:06 -05:00
Jason Ross d354f6470c Merge pull request #166 from JMR-dev/docs/coverage-wave2
Re-measure coverage after wave 2, and write down how a stacked PR merges
2026-08-29 11:33:20 -05:00
JMR-devandClaude Opus 5 dbedfb4708 Re-measure coverage after wave 2, and write down how a stacked PR merges
COVERAGE. 87.1% line / 69.1% branch, 502 tests -> 88.9% line (2087/2348), 75.4%
branch (1011/1340), 546 tests in 76 classes, as #153's five children land.

The branch figure moved for two reasons and the entry now says so, because only
one of them is new tests. The numerator rose 974 -> 1011; the denominator *fell*
1410 -> 1340. Both are the seam work: pulling a `when` out of a lambda inside a
`collect` deletes the coroutine state machine's synthesized branches around it,
and leaves a plain function whose branches a test can choose.
`ConversionViewModel$observe$1$1` went from carrying the whole mapping to six
branches, while the extracted `ConversionViewModelKt` covers 41 of 42 and
`JoinViewModelKt` 38 of 39.

That is worth stating rather than quoting the percentage alone. A number that
rises because the denominator shrank is a different claim from one that rises
because more branches are tested, and this entry has a documented history of
explaining its own movements wrongly.

STACKED PRS. A new Conventions entry, from two traps measured on 2026-08-27
while landing #144-#151.

`gh pr merge` refuses a stacked PR outright -- "must be merged using the
asynchronous merge REST API" -- and so does the plain `/merge` endpoint. The one
that works is `PUT .../pulls/N/merge-async`, which returns a uuid to poll.

The second is worse because nothing looks wrong. GitHub retargets a stacked PR's
base to main when the one below it merges, but asynchronously. Merging five about
thirty seconds apart outran it, so each merged into its own already-merged base
branch. Every call returned `status: merged`, every PR read MERGED, `gh pr list
--state open` was empty, and none of the content was on main.

What caught it was a coverage re-measure two points below what the same tree had
produced an hour earlier -- a fresh `git pull` changed nothing, which is what
made it a question rather than a stale checkout. `git merge-base --is-ancestor`
answers it in one line. #160 is what the recovery cost.

Also recorded: the auto-retarget belongs to the stacking feature. A PR opened
with a plain `--base some-branch` does not retarget when that branch merges, and
has to be moved by hand -- which is what #163 needed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 11:16:20 -05:00
Jason Ross c6e9a480e1 Merge pull request #165 from JMR-dev/test/screen-wiring
W3: the screen wiring, and a narrower hazard than the ticket claimed
2026-08-29 11:13:34 -05:00
Jason Ross d8110d7a62 Merge pull request #164 from JMR-dev/test/viewmodel-setters
W4: the seven settings edits, and three tests that did not bite until they did
2026-08-29 10:57:14 -05:00
JMR-devandClaude Opus 5 6f3966cc69 W3 (#156): the screen wiring, and a narrower hazard than the ticket claimed
The stateful outer composables hand `ConverterScreenContent` and
`JoinScreenContent` a list of `viewModel::` references. No test in the suite had
ever seen that list: the content tests build their own `ConverterActions`, so
they drive the stateless inner and never touch the wiring.

THE TICKET'S PREMISE WAS HALF WRONG, AND CHECKING BEAT ASSUMING. #156 was filed
claiming a transposition of any two of seventeen bindings would survive the
suite. Measured instead of trusted:

  onVideoCodec <-> onAudioCodec   -> REJECTED: "Inapplicable candidate(s):
                                     fun setAudioCodec(codec: AudioCodec)"
  onCancel     <-> onReset        -> COMPILES

Every typed binding -- container, both codecs, preset, suggestion, quality,
engine preference -- takes a distinct parameter type, so the compiler is already
the test. Writing assertions against those transpositions would have been
theatre, and this file says so rather than quietly including them.

WHAT IS ACTUALLY AT RISK is the `() -> Unit` bindings, which are interchangeable
to the compiler: two on the converter screen (onCancel, onReset) and *three* on
the join screen (onJoin, onCancel, onReset). A Cancel button that discards the
finished file, a Start-over that leaves it on screen, or a Join button that
cancels -- each is one wrong word and each ships.

I got that wrong in the first check too: an early run reported the onCancel/
onReset swap as rejected, from a grep-and-exit-code test that misread a stale
build. Re-running it properly printed BUILD SUCCESSFUL with the swap in place.

THE SEAM. `converterActions(viewModel, onPickInput, onConvert, onSave)` and
`joinActions(viewModel, onPickInputs, onSave)`. The launcher-backed actions stay
parameters -- they need an ActivityResultLauncher, which is the part that
genuinely needs a composition, and keeping them out means the rest needs none.

Told apart by effect rather than by a recording double: `reset()` sets the state
to Idle, `cancel()` with no active job leaves it alone (`activeWorkId?.let`,
which SettingsEditsTest pins).

Mutations -- every transposition caught, each by two tests:

  converter onCancel <-> onReset  | 2 tests
  join      onJoin   <-> onCancel | 2 tests
  join      onReset  <-> onCancel | 2 tests
  a typed binding dropped to {}   | 1 test
  a launcher action rerouted      | 1 test

The two-test symmetry is deliberate: one direction alone passes against a wiring
with BOTH actions bound to the same method, which is what a copy-pasted line
produces.

Three guard assertions earned their place during writing -- the picks land
through an injected dispatcher, and without `ParkedPickDispatcher.runAll()` all
three state-based tests sat on Idle and would have asserted nothing. They failed
loudly instead of passing quietly.

`@UnstableApi` on both builders, per CLAUDE.md; lint caught their absence, as it
did in W1.

525 -> 537 tests, 88.0% -> 88.9% line, 70.5% -> 75.4% branch. Gate green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 10:56:44 -05:00
Jason Ross 7595177e81 Merge pull request #163 from JMR-dev/test/join-state-mapping
W2: the join state mapping, and a crash the seam exposed
2026-08-29 10:49:03 -05:00
JMR-devandClaude Opus 5 a507736d3d W4 (#157): the seven settings edits, and three tests that did not bite until they did
`setPreset` was covered; the six beside it and `cancel()` had no coverage at all.
That asymmetry is the tell -- they are reachable from the JVM suite by exactly
the route `setPreset` already takes, and nothing had asked.

WHAT IS ASSERTED. Not "the setter sets something". Each of these copies into a
nested `OutputSpec`, so the failure worth catching is a setter that writes the
right value into the wrong field, or that rebuilds the spec and quietly discards
the other two. Every test asserts the field it changed AND that the rest survived.

THREE OF THEM DID NOT BITE, AND THE REASON IS WORTH KEEPING. The first run of the
mutations came back with two green:

  setContainer rebuilding from OutputFormat.MP4_H265.spec   -> GREEN
  setQuality also resetting enginePreference to AUTO        -> GREEN

Both for one mistake of mine: I asserted "the rest survived" against values that
were still at their defaults. `ConversionSettings` starts at `MP4_H265.spec`,
`QualityTier.FAST` and `EnginePreference.AUTO` -- so a mutation that RESET a
neighbouring field to its default was indistinguishable from one that left it
alone. The tests were checking a value, not a behaviour.

Fixed by moving each neighbour off its default before the call under test. A
third test had the same latent hazard -- it asserted `quality == FAST` -- and was
corrected with the others rather than left to fail later.

That is precisely the shape CLAUDE.md warns about ("five of them passing the
whole suite over a completely unguarded code path"), and it is the second time in
this wave the mutation pass has earned its place: green was not evidence.

Eight mutations, all red after the fix:

  setContainer rebuilds from a preset            | 1 test
  setQuality resets the engine preference        | 1 test
  setEnginePreference resets the quality         | 2 tests
  applySuggestion resets quality                 | 1 test
  setVideoCodec writes nothing                   | 1 test
  setAudioCodec writes nothing                   | 2 tests
  setEnginePreference writes nothing             | 2 tests
  cancel() dereferences a null activeWorkId      | 1 test

516 -> 525 tests, 87.7% -> 88.0% line, 70.4% -> 70.5% branch. Gate green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 10:46:48 -05:00
JMR-dev 1ff5c4463c Merge remote-tracking branch 'origin/main' into test/join-state-mapping 2026-08-29 10:39:55 -05:00
Jason Ross 70337b2c12 Merge pull request #162 from JMR-dev/test/conversion-state-mapping
W1: cut the conversion state mapping into a seam, and choose all six arms
2026-08-29 10:38:52 -05:00
JMR-devandClaude Opus 5 fea480b000 W2 (#155): the join state mapping, and a crash the seam exposed
The join-side twin of W1, deliberately the same shape -- one refactor done twice,
and letting the two diverge would cost more than the duplication. Five arms had
never been chosen by any test, for the same reason: a real ConcatWorker only ever
reaches a terminal state with well-formed output.

WHAT THE SEAM TURNED UP. This line was in the SUCCEEDED arm:

    info.outputData.getString(ConcatWorker.KEY_STRATEGY)
        ?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE

`valueOf` throws IllegalArgumentException on a name this build does not define,
and this runs inside a `viewModelScope` collect with no handler -- so it is not a
Failed state, it takes the process down.

Not theoretical. WorkManager keeps finished work about a week, so a downgrade or
rollback hands this build a job enqueued by another one -- the premise
`WorkerEnumFallbackTest` and `JobTags` are both written on. `ConcatWorker` writes
`result.strategy.name`, so a build that added a third strategy would leave this
one crashing on its own completed joins.

The codebase had already made this exact fix one file over, and said why:

    // Looked up rather than `valueOf` -- see the same three reads in
    // ConversionWorker. This one is above the try as well, so a format name this
    // build does not define used to throw past the catch

The matching read on the ViewModel side had not been changed with it. It is now
`ConcatStrategy.entries.firstOrNull { it.name == name } ?: REENCODE`.

PROVEN RATHER THAN ASSERTED. Restoring `valueOf` and running the new test:

    RED: an unknown strategy name is read as a re-encode rather than thrown
      java.lang.IllegalArgumentException: No enum constant
      org.libremediaconverter.model.ConcatStrategy.SMART_CONCAT_V2

REENCODE is the conservative default rather than an arbitrary one: it is the
answer for inputs that do not match, so a job whose strategy cannot be read is
described as the more cautious of the two rather than claimed as a lossless
stream copy. The mutation to STREAM_COPY reddens two tests.

Eight mutations, all red:

  unknown strategy -> STREAM_COPY            | 2 tests
  runAttemptCount ignored, both directions   | 2 tests
  success with no path -> empty Joined       | 1 test
  BLOCKED unfolded from RUNNING              | 1 test
  blank error no longer falls back           | 1 test
  cancellation ignores the caller's state    | 1 test

516 -> 530 tests, 87.7% -> 88.0% line, 70.4% -> 71.3% branch. Gate green:
assembleDebug, testDebugUnitTest, compileDebugAndroidTestKotlin, ktlintCheck,
detekt, lintDebug.

Stacked on W1 (#162), which this mirrors and should not land before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 10:37:13 -05:00
JMR-devandClaude Opus 5 ddfb1dd78e W1 (#154): cut the conversion state mapping into a seam, and choose all six arms
`ConversionViewModel.observe` maps a `WorkInfo` onto a `ConversionState`. That is
the app's main UI state machine, and no test had ever chosen which arm it took.

NOT COLD CODE, WHICH IS THE POINT. `ConversionViewModel$observe$1$1` already
reported 28 covered lines and 24 covered branches: every test that drives a real
worker runs this. But a real worker only ever reaches a terminal state with
well-formed output, so `SUCCEEDED`-with-a-path and `FAILED`-with-a-message were
the only arms any test had produced. The other six ran never -- the progress
read, both sides of the retry check, a success naming no file, a failure with
nothing to say, `CANCELLED`, and `BLOCKED`.

A grep makes that look untrue: all six `WorkInfo.State` constants appear in the
JVM suite. They are in `ReattachmentTest`, driven into `Reattachment.choose` --
a *different* function encoding the same enqueued-means-retry rule. So the rule
had a test in one of its two homes, and the copy the user's screen reads had
none.

THE SEAM. `workManager` comes from `WorkManager.getInstance` in the constructor
and `observe` is private, so nothing could hand this a chosen `WorkInfo`. The
`when` is now `conversionStateFrom`, a pure function over a `ConversionUpdate`
carrying only the fields it reads -- the same shape as `JobSnapshot` beside
`Reattachment.choose`, and its KDoc gives the same reason. `outputData` stays a
`Data`, which this suite already builds with `workDataOf` everywhere; unpacking
it into five nullable strings would move the same reads without helping.

TWO THINGS DELIBERATELY LEFT OUTSIDE IT:

  - The ownership check stays at the call site. Its comment says it guards the
    file ownership the SUCCEEDED arm takes, not merely the assignment, so moving
    it inside would change what it protects.
  - The mapping takes no responsibility for the staged file. It returns the
    state; the caller reads the file off the result. That is strictly better
    than the original, where `pendingStaged = staged` happened inside one arm:
    "the state and `pendingStaged` refer to the same file or to no file" is now
    the shape of the code rather than a rule two branches have to keep.

Mutations, each killing exactly the test it should:

  | mutation                                    | red test                    |
  |---------------------------------------------|-----------------------------|
  | progress read ignored                       | reports the progress        |
  | runAttemptCount ignored -> always Waiting   | never run is simply starting|
  | runAttemptCount ignored -> never Waiting    | already run is waiting      |
  | success with no path -> empty Converted     | named no file is a failure  |
  | blank name/type no longer falls back        | blank falls back like missing|
  | blank error no longer falls back            | blank message falls back    |
  | cancellation ignores the caller's state     | lands where caller said     |
  | BLOCKED remapped                            | blocked looks like starting |

`ENQUEUED` needs both mutations and both tests: either one alone passes against a
mapping that ignores `runAttemptCount` entirely.

The extracted functions carry `@UnstableApi` rather than swallowing the marker
with `@OptIn`, per CLAUDE.md -- lint's UnsafeOptInUsageError caught their absence.
An early `@Suppress("ReturnCount")` turned out to be unnecessary and was removed
rather than left: detekt is clean without it, and the file now carries none.

502 -> 516 tests, 87.1% -> 87.7% line, 69.1% -> 70.4% branch. Gate green:
assembleDebug, testDebugUnitTest, compileDebugAndroidTestKotlin, ktlintCheck,
detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 10:23:52 -05:00
Jason Ross 348eaa2f22 Merge pull request #161 from JMR-dev/test/dedupe-user-messages
W5: one sentence per user-facing condition, not two
2026-08-29 10:14:08 -05:00
JMR-devandClaude Opus 5 104d02de03 W5 (#158): one sentence per user-facing condition, not two
Four messages were written out in two places each, in a codebase that already
had the convention for this and states it in `OutputPublisher.kt`:

    Kept next to [STAGED_FILE_GONE_MESSAGE] for the same reason it is: both
    ViewModels need it and staging is what it is about.

The ticket named three. A wider scan -- `"[A-Z][^"]{8,90}[.!]"` rather than the
{15,70} that produced the original list -- found a fourth, `"Joining failed."`,
which is the exact join-side twin of `"Conversion failed."` and had been missed
because it is fifteen characters long.

  "Pick at least two files to join."  ->  ConcatWorker.TOO_FEW_INPUTS_MESSAGE
  "Joining failed."                   ->  ConcatWorker.GENERIC_FAILURE_MESSAGE
  "Conversion failed."                ->  ConversionWorker.GENERIC_FAILURE_MESSAGE
  "Could not save the file."          ->  SAVE_FAILED_MESSAGE, beside
                                          STAGED_FILE_GONE_MESSAGE

Each constant sits with the layer that owns the condition, which is what the two
existing constants do. The arity rule is the worker's -- `request(...)` takes a
`List<Uri>` and checks nothing about its length -- so `TOO_FEW_INPUTS_MESSAGE`
lives there and the ViewModel reads it, not the other way round.

WHY THE TWO `Log.e` LITERALS STAY. `"Conversion failed."` and `"Joining failed."`
each also appear in a log line beside the failure they describe. Those keep their
own copies: a log has a different audience and carries the exception with it, and
coupling it to the user-facing wording would mean rewording the screen to change
a log. Stated in the KDoc so the next scan does not read them as a miss.

THE TEST IS A CROSS-LAYER ONE, DELIBERATELY. #158's done-when is explicit that "a
test asserting the constant equals its own value is worth nothing". Sharing a
constant makes the two sites agree by construction; what it cannot show is that
both layers still *reach* it. So `SharedFailureMessagesTest` drives each for real
-- the ViewModel through `onInputsPicked`, the worker through `doWork` -- and
asserts the two answers are the same string, taken from two running layers rather
than from one declaration.

That the sharing was worth doing at all is visible in what was pinned before:
`RefusedJobTest` (#139) pinned the worker's copy of the arity message and nothing
pinned the ViewModel's, so the screen's wording could drift with no test saying
anything.

Mutations:

| mutation | result |
|---|---|
| ViewModel keeps its own drifted literal | red |
| ViewModel's arity guard removed entirely | red |

Gate green: assembleDebug, testDebugUnitTest, compileDebugAndroidTestKotlin,
ktlintCheck, detekt, lintDebug.

Not done here: `"Saved ${s.displayName}."` appears in both screens. It is left
alone, and the reason is a real distinction rather than an oversight -- the four
above are cases where one layer's message is another layer's *fallback*, so drift
means the user sees different words for one condition. Two screens each wording
their own success text is ordinary UI, and drift there is cosmetic.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 10:05:27 -05:00
Jason Ross 90814222b7 Merge pull request #160 from JMR-dev/fix/restore-stack-merges
Restore the five stacked PRs that merged into their bases instead of main
2026-08-29 09:09:46 -05:00
JMR-devandClaude Opus 5 2d4898ad44 Re-measure the coverage entry against the tree this branch creates
84.9% line / 63.8% branch, 454 tests -> 87.1% line (2025/2324), 69.1% branch
(974/1410), 502 tests in 71 classes, as #132 and #133's ten children land.

The entry already instructs re-measuring before quoting, and that is why this is
here rather than in the batch: quoting these numbers before the work merged would
have described a tree that did not exist. It nearly went wrong the other way too
-- the first measurement for this commit was taken against a main that was three
merges stale and read 85.1%.

Also says something the bare numbers do not. Branch moved 5.3 points against
line's 2.2, and that asymmetry is the expected shape of this kind of work rather
than a curiosity: those children targeted decision code -- enum fallbacks,
refusal arms, cursor shapes, a `when` over container rules -- where one test
chooses a branch the suite had never taken. Line coverage barely notices that.
Branch coverage is the whole point.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-27 09:01:40 -05:00
JMR-dev 83ac7eff2c Merge commit '79cca0e' into fix/restore-stack-merges 2026-08-27 08:59:45 -05:00
Jason Ross b2c11bbfe0 Merge pull request #151 from JMR-dev/test/outputpublisher-seams
S2 + S3: the two OutputPublisher seams, and where the second one actually goes
2026-08-27 08:54:30 -05:00
JMR-devandClaude Opus 5 3a5210ec5d S2 + S3 (#142, #143): the two OutputPublisher seams, and where the second one goes
#142 -- openOutputStream refuses two ways and only one was reachable. A
provider that has gone away throws from inside the call, which
`a destination the provider will not open...` already drives. A provider
that is present and declines returns null, and nothing could produce that
on demand. openDestination is the seam; the test asserts the failure names
the destination, which is what separates the `?: error(...)` from an NPE
inside `use`.

#143 -- the sweep's re-read. **The seam the ticket proposed does not reach
it.** Overriding the listing fires before the entries are snapshotted, so
StagingSweep.collectable is handed the new timestamp, the file is never
proposed for deletion, and the guard is never exercised. Measured: with an
entriesIn seam, deleting the guard outright left the test green.

The race is a file that *was* collectable when the snapshot was taken and
is not by the time the delete comes round, so the seam has to sit at the
snapshot. `snapshot(listing)` does, and deleting the guard now reddens the
test.

Three mutations after the move, three red:

  null stream returns silently   null-return test
  null stream via !! instead     null-return test
  sweep deletes unconditionally  race test

OutputPublisher.kt now has no never-executed lines at all. Two partial
branches are left and both are named exemptions rather than gaps:
L216's `getOrNull() ?: false` and L304's `getOrDefault(absoluteFile)` are
the failure arms of a runCatching whose body cannot be made to throw
through any public entry point -- the same shape as the `size >= 0`
exemption recorded in the previous commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-27 07:22:38 -05:00
Jason Ross 5f3eda9c40 Merge pull request #149 from JMR-dev/test/outputpublisher-partial-branches
C6: OutputPublisher's guarded branches, three of them guarding a delete
2026-08-27 07:22:35 -05:00
JMR-dev 79cca0eb47 Merge branch 'test/refused-jobs' into test/mediaprobe-track-seam 2026-08-27 07:19:42 -05:00
JMR-dev 2c0bc4a583 Merge branch 'test/concatworker-failure-arms' into test/refused-jobs 2026-08-27 07:19:41 -05:00
JMR-dev 7a47285f37 Merge branch 'test/container-capabilities-audio' into test/concatworker-failure-arms 2026-08-27 07:19:39 -05:00
JMR-dev 8a2bc86cac Merge branch 'test/readspec-enum-fallbacks' into test/container-capabilities-audio 2026-08-27 07:19:38 -05:00
JMR-dev 9b3b9f952b Merge remote-tracking branch 'origin/test/outputpublisher-seams' into test/readspec-enum-fallbacks 2026-08-27 07:19:37 -05:00
JMR-dev a84b24ba27 Merge branch 'test/refused-jobs' into test/mediaprobe-track-seam 2026-08-27 07:18:29 -05:00
JMR-dev 0e2525195b Merge branch 'test/concatworker-failure-arms' into test/refused-jobs 2026-08-27 07:18:28 -05:00
JMR-dev 713d813a65 Merge branch 'test/container-capabilities-audio' into test/concatworker-failure-arms 2026-08-27 07:18:27 -05:00
JMR-dev c360e82a10 Merge branch 'test/readspec-enum-fallbacks' into test/container-capabilities-audio 2026-08-27 07:18:25 -05:00
JMR-dev 699d608b47 Merge remote-tracking branch 'origin/main' into test/readspec-enum-fallbacks 2026-08-27 07:18:24 -05:00
JMR-devandClaude Opus 5 ad47ce6c96 S2 + S3 (#142, #143): the two OutputPublisher seams, and where the second one goes
#142 -- openOutputStream refuses two ways and only one was reachable. A
provider that has gone away throws from inside the call, which
`a destination the provider will not open...` already drives. A provider
that is present and declines returns null, and nothing could produce that
on demand. openDestination is the seam; the test asserts the failure names
the destination, which is what separates the `?: error(...)` from an NPE
inside `use`.

#143 -- the sweep's re-read. **The seam the ticket proposed does not reach
it.** Overriding the listing fires before the entries are snapshotted, so
StagingSweep.collectable is handed the new timestamp, the file is never
proposed for deletion, and the guard is never exercised. Measured: with an
entriesIn seam, deleting the guard outright left the test green.

The race is a file that *was* collectable when the snapshot was taken and
is not by the time the delete comes round, so the seam has to sit at the
snapshot. `snapshot(listing)` does, and deleting the guard now reddens the
test.

Three mutations after the move, three red:

  null stream returns silently   null-return test
  null stream via !! instead     null-return test
  sweep deletes unconditionally  race test

OutputPublisher.kt now has no never-executed lines at all. Two partial
branches are left and both are named exemptions rather than gaps:
L216's `getOrNull() ?: false` and L304's `getOrDefault(absoluteFile)` are
the failure arms of a runCatching whose body cannot be made to throw
through any public entry point -- the same shape as the `size >= 0`
exemption recorded in the previous commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-27 07:14:19 -05:00
JMR-devandClaude Opus 5 c60d5d54c6 Stop the staging fixture losing a race with the app-start sweep (#159)
`the sweep tolerates a staging path that is not a directory` failed once on run
33069641674, against 468 tests that pass on this machine including under
`--rerun-tasks`:

    java.io.FileNotFoundException at OutputPublisherStagingTest.kt:112
    468 tests completed, 1 failed

Line 112 was `writeBytes` immediately after `deleteRecursively()`.
`FileOutputStream` answers `FileNotFoundException` for an existing directory, so
something had recreated the path inside that window. That something is
`LibreMediaConverterApp.onCreate`, which ends with

    appScope.launch { OutputPublisher(...).sweepStaging() }

on `Dispatchers.IO`, and `sweepStaging` reads `stagingDir`, whose getter calls
`mkdirs()`. Robolectric builds the application for every test that asks for one,
so that background `mkdirs()` is in flight across the whole suite on a thread the
paused main looper does not control and no test awaits.

Retrying closes the window rather than narrowing it, because the race is not
symmetric: `mkdirs()` fails on an existing regular file, so the invariant only has
to survive being *established*. Once a write lands, nothing in the suite can turn
this path back into a directory -- which is also why the new assertion that the
sweep left a file behind is worth making.

The `check()` matters as much as the loop. The next failure here should say
"something recreated conversions/ as a directory", not `FileNotFoundException at
line 112` -- that is the difference between a flake someone reads and a flake
someone re-runs.

The wider problem is #159 and is deliberately not fixed here: `AppStartSweepTest`,
`JobSnapshotsTest` and `SpaceArithmeticTest` all name the same path, and the real
answer is an injectable scope rather than a retry loop in every staging test.
#159's done-when is that this loop can be deleted.

Mutation: `listFiles() ?: return` -> `listFiles()!!` reddens exactly this test.
Gate green: assembleDebug, testDebugUnitTest, compileDebugAndroidTestKotlin,
ktlintCheck, detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-27 07:14:18 -05:00
JMR-devandClaude Opus 5 d59e9acce5 C6 (#140): OutputPublisher's guarded branches, three of them guarding a delete
destinationIsKnownEmpty's three short-circuits -- no SIZE column, no row,
a null cell -- each had to answer false and none was tested. Its KDoc is
unambiguous about why: "this decides whether a delete is allowed and 'I
could not tell' must never authorise one." The existing tests only ever
drove a provider that answers properly, where the answer is zero and the
delete is correct. Getting the uncertain cases backwards costs the user a
file they already had, on a save that failed.

Also discardStaged's null parentFile, and sweepStaging's null listing --
which is not the case the existing `tolerates a staging directory that
does not exist yet` covers, because stagingDir's own mkdirs() recreates a
missing directory and it then lists as empty. Only a path that cannot be
a directory makes listFiles() answer null.

Five mutations, three bite:

  drop !row.isNull(size)                 short-circuit test red
  drop row.moveToFirst()                 five tests red
  parentFile!! instead of ?: return false parentless test red

  size >= 0  ->  size >= -1              GREEN, does not bite
  parentless treated as staged           GREEN -- bad mutation, see below

The first green one is recorded in the test as a named exemption.
Measured: getColumnIndex returns -1 for an absent column and isNull(-1)
throws CursorIndexOutOfBoundsException, which the surrounding runCatching
already turns into `?: false`. Same answer, reached by the exception path,
so no behavioural test can pin that conjunct. It stays anyway -- control
flow through an exception is worse than a comparison, and another Cursor
implementation need not throw.

The second was my mistake rather than a finding: substituting stagingDir
for the null parent reaches `return false` by a different route, so it
proves nothing. parentFile!! is the honest mutation and it goes red.

:197, :235 and :258 are now covered. What is left in this file is exactly
what the ticket scoped out: :173-174 (#142) and :267 (#143).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-27 07:14:18 -05:00
Jason Ross 324c9a4555 Merge pull request #144 from JMR-dev/test/fake-provider-scaffolding
C0 + C3: fake-provider scaffolding, and what InputQuery makes of a metadata row
2026-08-27 07:14:14 -05:00
JMR-devandClaude Opus 5 44493d9943 C5 (#139): the join side's count refusal, found by the residual-gap audit
A gap audit over the eight branches merged together looked for lines still
never executed and asked, for each, whether something already accounts for it.
Everything mapped except one: `ConcatWorker.kt:42`, the refusal of a join with
fewer than two inputs.

Its neighbour maps. `ConcatWorker.kt:40` -- the missing-URI-array arm, two lines
above -- is covered on the device by
`UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage`. That is invisible to
JaCoCo, which measures `testDebugUnitTest` only, so the report shows both arms
cold and cannot distinguish the one that is e2e-covered from the one nothing
touches. Only reading the androidTest source separates them.

`grep` says nothing in either source set mentions "Pick at least two files to
join." Two tests here now do:

- `a join of a single file is refused with a message rather than joined` pins
  the verdict and the message together, via `Failure.equals`, for the reason the
  file's header already gives.
- `a join of two files is not refused for its count` is the control that puts
  the assertion on the boundary rather than on the string. It refuses the
  *space* rather than letting the job run: the next thing past the count guard
  is `ConcatEngine`, which is native, and `NamingPublisher`'s KDoc already
  records that no JVM test gets past it. A failure carrying the space message is
  proof execution reached line 57, which is proof it cleared line 42, at no
  engine cost.

Reachability is the header's argument plus one of its own: `request(...)` takes
a `List<Uri>` and checks nothing about its length, so a one-item join is a
well-formed call rather than a corrupted queue entry.

Mutations, each killing exactly the test it should:

| mutation | red |
|---|---|
| guard deleted outright | `a join of a single file is refused...` |
| `uris.size < 2` -> `< 3` | `a join of two files is not refused for its count` |

Restored, both green. Gate green: assembleDebug, testDebugUnitTest,
compileDebugAndroidTestKotlin, ktlintCheck, detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 23:14:46 -05:00
JMR-devandClaude Opus 5 b2790e13d9 S1 (#141): cut the track walk into a pure seam, and test the matrix
#84 closed by classifying probeWithExtractor and probeForConcat as
device-bound and explicitly not a gap. That was right about FFprobe and
right about the measurement boundary, and wrong that these are only
orchestration. The track walk is a branch matrix, and androidTest reaches
it only through whatever the committed fixtures happen to contain -- so
none of its rules is *chosen* by any test there.

#133 offered two ways to reach it: drive ShadowMediaExtractor, or cut the
loop into a pure function. Taking the second, which is the pattern
CLAUDE.md names and work/FailureOutcome.kt documents. extractedFrom and
concatInputFrom take List<MediaFormat>; what is left needing a device --
setDataSource, getTrackFormat, release -- is one three-line extension
function, which is the thin edge androidTest should be covering.

The two are deliberately not merged despite the overlap. One reads
duration and not frame rate; the other reads frame rate and not duration.
A merged version would compute both for every caller, and ConcatPlanner
treats an unknown frame rate as "cannot prove a match" -- so a field the
join flow does not need must not start arriving as a number.

Eleven tests over cases no fixture provides: two video tracks, two audio
tracks, audio outlasting video, a track with no KEY_DURATION, audio
declared before video, a subtitle track, and no tracks at all.

Six mutations, six red:

  last video track wins            first-video test
  last audio track wins            first-audio test
  duration = last rather than max  longest-track test
  drop the containsKey guard       six tests (getLong throws on a missing key)
  guess a frame rate of 30         no-frame-rate test
  join takes the last video track  join frame-rate test

MediaProbe's missed branches drop 91 -> 70; what is left is the FFprobe
half and the two catch arms, which are native and device-bound exactly as
#84 said.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 22:51:10 -05:00
JMR-devandClaude Opus 5 de6d9526ba C5 (#139): the two jobs ConversionWorker refuses before converting
Both exits were cold, and both are reachable for the same reason: a job
does not have to come from the picker. WorkManager keeps work for about a
week, so a downgrade or rollback hands this build a job enqueued by
another one, and request(...) is callable directly.

:62 -- a missing KEY_INPUT_URI -- was untested everywhere, JVM and device.
The nearest e2e test, ForcedFailureTest.aMissingInputFailsRatherThanCrashing,
passes a URI pointing at a file that does not exist, which reaches the
engine and fails much later with a different message.

:124-126 -- the Validation.Invalid refusal -- had no test at all, though
its comment names both arrival paths it exists for.

Five tests, in a new file because both are about the *message*. A refusal
that fails with empty output Data renders the UI's generic "Conversion
failed." with nothing else to say, which is the defect shape
DeniedForegroundStartTest records from the device pass; asserting the
verdict alone would pass against exactly that.

Two of the five are there to stop the others passing for the wrong reason:
`a refused spec never reaches an engine` says it failed *before*
converting rather than during, and `a valid spec is not refused` is the
control -- without it every assertion here would still pass against a
worker that refused everything.

Three mutations, three red:

  change the no-input message         no-input message test
  drop the validation refusal         both refusal tests
  validate but keep converting        both refusal tests

L61-62 and L123-126 are now fully covered, branches included.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 22:37:23 -05:00
JMR-devandClaude Opus 5 bb3358f209 C4 (#138): ConcatWorker's cancellation and give-up arms
ConversionWorker has WorkerCancellationTest and DeniedForegroundStartTest.
Its twin had the retry case only -- `a join whose foreground start is
denied` already existed -- so two of ConcatWorker's three failure exits
were cold: the CancellationException arm, and FOREGROUND_DENIED.

Four tests, added to the files that own each rule rather than to a new
ConcatWorker file, which is how this suite is organised: a file per rule,
tested across both workers.

The cancellation seam is worth a look in review. The conversion twin
cancels inside the engine, which is honest there because
ConversionDependencies has a seam for it. ConcatWorker calls ConcatEngine
directly and has none -- it is native and nothing here gets past it -- so
the cancellation is injected at the only other point inside the try,
setForeground. That is a real shape rather than a contrivance: a job
cancelled while WorkManager is promoting it is exactly when that window is
open, and the catch arm cannot tell where in the try it came from.

FailedFuture moved to WorkerStubs.kt on the way. Two tests now inject two
different failures through it, and Kotlin will not take two file-private
top-level classes of one name in one package.

Four mutations, four red, each isolated:

  cancellation arm -> Result.failure     propagation test only
  drop delete on cancellation            cancellation-partial test only
  FOREGROUND_DENIED -> Result.retry      past-the-bound test only
  drop delete on the Throwable path      give-up-partial test only

ConcatWorker's :92, :95-96 and :105-106 are covered; missed branches 4 -> 3.
What is left is what the ticket scoped out: the two input guards (e2e), the
ConcatEngine success path (native), and getForegroundInfo (#88's named
exemption).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 22:33:04 -05:00
JMR-devandClaude Opus 5 04850a0415 C2 (#136): test the audio half of validate, and the one video refusal missing
The two halves of ContainerCapabilities.validate were written together
and only one of them was ever checked. Six audio outcomes had no test --
every one a string the user reads -- while the video twin of each was
already covered.

Seven tests, deliberately shaped like their twins rather than as a fresh
idea about what to assert:

  unidentifiable source audio on a COPY   twin of `an unidentifiable
                                          source codec cannot be copied`
  container cannot hold the copied source twin of `a codec the container
                                          cannot hold is refused...`
  container cannot carry it on encode     twin of `H265 in AVI is refused`
  this app cannot encode it               twin of `copying is offered as
                                          the fix when...`
  accepts(_, AudioCodec.NONE, _) -> true  twin of the VideoCodec.NONE arm
  accepts(_, AudioCodec.COPY, _) throws   twin of `resolving COPY before
                                          asking the matrix is required`

The seventh is not the audio axis: validateVideo's copy-into-a-container-
that-cannot-hold-it refusal was the one video outcome with no test, and it
is the same shape and the same file.

Each asserts the message verbatim and re-validates every suggestion the
refusal offers. Validation.Invalid promises its suggestions are themselves
valid and names this class as the proof; the existing property test walks
the presets, and no preset reaches suggestions() through validateAudio.

Seven mutations run, seven red, each isolated to exactly one test:

  CARRIES_AUDIO check -> false     encode-path test only
  drop the COPY error arm          resolve-first test only
  AudioCodec.NONE -> false         no-audio-track test only
  drop ENCODABLE_AUDIO check       unencodable test only
  drop audio copy container check  audio-copy test only
  drop video copy container check  video-copy test only
  drop unidentified-audio guard    unidentifiable test only

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 22:27:27 -05:00
JMR-devandClaude Opus 5 8ab433b647 C1 (#135): pin readSpec's three enum fallbacks
WorkerEnumFallbackTest already existed for this defect class -- a name
this build does not define, read above the try, throwing out of doWork
entirely: FAILED with reschedule=false, empty output Data so the screen
said "Conversion failed." with nothing else, and the staged file never
deleted. It covered 2 of the 5 above-the-try reads. readSpec's three
were the ones left, and all three were cold.

The baseline is the part worth reviewing. readSpec returns the *entire*
fallback spec the moment any one axis fails to resolve, so a test
starting from MP4_H265 -- which is itself the fallback -- cannot tell a
worker that read the spec correctly from one that gave up on it. These
start from MKV/H.264, which differs on container and video codec at
once, and assert the spec that actually reached the transcoder rather
than only that a Result came back.

Mutations run, four for three tests:

  KEY_CONTAINER    `?: return fallback` -> `?: error(...)`  -> container test red
  KEY_VIDEO_CODEC  same                                     -> video test red
  KEY_AUDIO_CODEC  same                                     -> audio test red
  fallback = MP4_H264 instead of MP4_H265                   -> all three red

The first three confirm the tests are isolated to their own axis; the
fourth confirms they pin *which* spec ran, which is what "a Result at
all" would have missed.

readSpec is now fully covered, branches included.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 22:24:10 -05:00
35 changed files with 3449 additions and 220 deletions
+56 -7
View File
@@ -130,8 +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** — **84.9% of lines (1971/2321), 63.8% of branches**,
measured 2026-08-26 with `./gradlew :app:jacocoTestReport`, against 454 JVM tests in 67 classes.
- **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.
**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
@@ -147,12 +148,34 @@ install for code that can never run — and on API 37 the full APK does not fit
disproportionately Robolectric, so each one added denominator and no numerator — the measurement
was punishing exactly the tests that were hardest to write.
Two things still hold. A floor needs a baseline that has settled, and this one has not: it moved
39 points in a single build change on 2026-08-24, then another 16 as the #52 test push and the
Two things still hold. A floor needs a baseline that has settled, and this one has not. It moved
39 points in a single build change on 2026-08-24; then another 16 as the #52 test push and the
fixes it turned up landed — 69.2% -> 84.9% line, 53.2% -> 63.8% branch — while the denominator
grew 2194 -> 2321, because that work added production code of its own. And **re-measure before
quoting**: this entry was written quoting 81.4%, measured four hours earlier, and was already
three points stale by the time it was ready to merge.
grew 2194 -> 2321, because that work added production code of its own; then again on 2026-08-27
as #132 and #133's ten children landed — 84.9% -> 87.1% line, 63.8% -> **69.1%** branch, 454 ->
502 tests. **Branch moved four times as far as line that last time**, and that is the shape to
expect from this kind of work rather than a surprise: those children targeted decision code —
enum fallbacks, refusal arms, cursor shapes, a `when` over container rules — where one test
chooses a branch the suite had never taken. Line coverage barely notices; branch coverage is the
whole point.
Then #153's five children on 2026-08-29 — 87.1% -> 88.9% line, 69.1% -> **75.4%** branch, 502 ->
546 tests.
**That last branch figure moved for two reasons, and only one of them is new tests.** The
numerator rose 974 -> 1011; the denominator *fell* 1410 -> 1340. Both are the seam work. Pulling
a `when` out of a lambda inside a `collect` deletes the coroutine state machine's synthesized
branches around it, and what is left is a plain function whose branches a test can choose:
`ConversionViewModel$observe$1$1` went from carrying the whole mapping to 6 branches, while the
extracted `ConversionViewModelKt` covers 41 of 42 and `JoinViewModelKt` 38 of 39. So a seam is
worth more than the tests it enables — it also stops the measurement counting scaffolding.
Be careful quoting a branch move on its own for that reason. A percentage that rises because the
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.
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
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.
@@ -174,6 +197,32 @@ install for code that can never run — and on API 37 the full APK does not fit
- `kotlin.code.style=official`. Gradle stays Kotlin DSL.
- **A stacked PR does not merge with `gh pr merge`, and `MERGED` is not proof it reached `main`.**
Two separate traps, both measured on 2026-08-27 while landing #144-#151.
`gh pr merge` uses the GraphQL mutation, which refuses a stacked PR outright: *"This pull request
is part of a stack and must be merged using the asynchronous merge REST API."* So does
`PUT .../pulls/{n}/merge`. The one that works is
`gh api -X PUT repos/OWNER/REPO/pulls/N/merge-async -f merge_method=merge`, which returns a uuid
to poll at `.../merge-async/{uuid}` until `status` is `merged` or `failed`.
**The second trap is worse, because nothing looks wrong.** GitHub retargets a stacked PR's base
to `main` when the PR below it merges, but *asynchronously*. Merge a stack faster than that
settles — five PRs about thirty seconds apart, in the case that found this — and each one merges
into its own base branch, which has itself already been merged and left behind. Every call
returns `status: merged` and every one is true. `gh pr list --state open` comes back empty, every
PR shows `MERGED`, and **none of the content is on `main`**.
What caught it was a coverage re-measure reading two points lower than the same tree had measured
an hour earlier; a fresh `git pull` changed nothing, which is what turned it into a question.
`git merge-base --is-ancestor <merge-sha> origin/main` answers it in one line. Do that after
merging a stack, or merge one at a time and re-read `baseRefName` between. #160 is what the
recovery cost.
The auto-retarget belongs to the stacking feature specifically. A PR opened with a plain
`gh pr create --base some-branch` does **not** retarget when that branch merges — it is left
pointing at a dead base and has to be moved by hand.
- **File one-off issues with `tools/github/file-issue.sh`, not `gh issue create`.** `gh issue
create` does not touch the project board, so the issue exists, carries its labels, and is
invisible in the Kanban — indistinguishable from never having been filed. Measured 2026-08-24:
@@ -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,
@@ -6,6 +6,7 @@ import android.util.Log
import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.viewModelScope
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.WorkInfo
import androidx.work.WorkManager
import kotlinx.coroutines.CoroutineDispatcher
@@ -71,6 +72,119 @@ data class InputFile(
val probe: InputProbe? = null,
)
/**
* One update about a running conversion, as WorkManager last reported it.
*
* Only the fields [conversionStateFrom] reads — the same shape, and for the same reason, as
* `JobSnapshot` beside `Reattachment.choose`: the rule stays testable on the JVM because nothing
* in it needs a `WorkInfo`, which a test cannot readily build.
*
* [outputData] stays a `Data` rather than being unpacked into five nullable strings. It is what a
* test already builds with `workDataOf` everywhere in this suite, so unpacking would move the same
* reads without making anything easier to drive.
*/
internal data class ConversionUpdate(
val state: WorkInfo.State,
val progressPercent: Int,
val runAttemptCount: Int,
val outputData: Data,
)
/**
* What the screen should show, given what WorkManager last said about the job.
*
* ## Why this is a function rather than the body of a `collect`
*
* It was the body of one. `workManager` is built in the constructor from `WorkManager.getInstance`,
* `observe` is private, and nothing could hand either a chosen `WorkInfo` — so every arm below ran
* only when a real worker happened to produce it. A real worker produces a terminal state with
* well-formed output, which meant six of these arms had never been chosen by any test: the progress
* read, both sides of the retry check, a success with no file, a failure with nothing to say, and
* the two that map to a state the user cannot otherwise reach.
*
* That is the argument #141 made for `MediaProbe`'s track walk, against `WorkManager` instead of a
* media fixture, and it takes the same answer: the branch matrix is a pure function, and what is
* left needing the framework — the flow, the null check, the ownership check — is the thin edge.
*
* ## What is deliberately *not* in here
*
* The ownership check stays at the call site. Its comment is explicit that it guards the file
* ownership the `SUCCEEDED` arm takes, not merely the assignment, so moving it inside would change
* what it protects. And this function takes no responsibility for the staged file: it returns the
* state, and the caller reads the file off it. A pure function that deletes files is not a seam.
*
* @param cancelled where a cancellation lands, which differs for a reattached job — see [observe].
* @param fallbackSpec the current settings, read only when finished work predates the worker
* reporting its own name and MIME type.
*/
@UnstableApi
internal fun conversionStateFrom(
update: ConversionUpdate,
input: InputFile,
cancelled: ConversionState,
fallbackSpec: OutputSpec,
): ConversionState = when (update.state) {
WorkInfo.State.RUNNING -> ConversionState.Converting(input, update.progressPercent)
// ENQUEUED after a run means a retry is pending. Either the six-hour foreground budget ran out
// mid-job, or the system refused to let the job start again while the app was in the background
// — the second being the likelier of the two, since it needs only a process restart. Nothing
// here can tell them apart, and nothing needs to: the answer is the same.
WorkInfo.State.ENQUEUED ->
if (update.runAttemptCount > 0) {
ConversionState.Waiting(input)
} else {
ConversionState.Converting(input, 0)
}
WorkInfo.State.SUCCEEDED -> convertedFrom(update.outputData, input, fallbackSpec)
// A worker that dies before it can report anything leaves no output data at all — a
// foreground-service start refused after a process restart is one way — and an exception's
// message can be an empty string. Both would read as a failure with nothing said, so blank
// falls back like missing does.
WorkInfo.State.FAILED -> ConversionState.Failed(
update.outputData.getString(ConversionWorker.KEY_ERROR)
?.takeIf { it.isNotBlank() }
?: ConversionWorker.GENERIC_FAILURE_MESSAGE,
)
WorkInfo.State.CANCELLED -> cancelled
WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
}
/**
* The `SUCCEEDED` arm, which is the only one that reads more than one field.
*
* Split out so [conversionStateFrom] stays a table of one line per state. A success with no output
* path is a failure: the job said it finished and named nothing, and there is no file to offer.
*/
@UnstableApi
private fun convertedFrom(outputData: Data, input: InputFile, fallbackSpec: OutputSpec): ConversionState {
val path = outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)
?: return ConversionState.Failed(SUCCEEDED_WITHOUT_A_FILE_MESSAGE)
return ConversionState.Converted(
input = input,
staged = File(path),
engineUsed = outputData.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
routeReason = outputData.getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
// Work enqueued before the worker reported this carries nothing, and WorkManager keeps
// finished work for about a week -- so this branch is ordinary for a few days rather than a
// corner. It is the old derivation, kept because it is the same guess the app already made
// and there is genuinely nothing better available for such a job. New work never reaches it.
suggestedName = outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME)
?.takeIf { it.isNotBlank() }
?: ConversionWorker.outputNameFor(input.displayName, fallbackSpec),
mimeType = outputData.getString(ConversionWorker.KEY_MIME_TYPE)
?.takeIf { it.isNotBlank() }
?: fallbackSpec.mimeType,
)
}
/** A job that reported success and named no file. There is nothing to offer the user to save. */
internal const val SUCCEEDED_WITHOUT_A_FILE_MESSAGE: String =
"Conversion reported success but produced no file."
sealed interface ConversionState {
data object Idle : ConversionState
data class Ready(val input: InputFile) : ConversionState
@@ -442,75 +556,24 @@ class ConversionViewModel @JvmOverloads constructor(
// that either. The state and `pendingStaged` are meant to refer to the same file
// or to no file, and this is where that stays true.
if (!ownership.stillHeldBy(token)) return@collect
_state.value = when (info.state) {
WorkInfo.State.RUNNING -> ConversionState.Converting(
input,
info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
)
// ENQUEUED after a run means a retry is pending. Either the six-hour
// foreground budget ran out mid-job, or the system refused to let the job
// start again while the app was in the background — the second being the
// likelier of the two, since it needs only a process restart. Nothing here
// can tell them apart, and nothing needs to: the answer is the same.
WorkInfo.State.ENQUEUED ->
if (info.runAttemptCount > 0) {
ConversionState.Waiting(input)
} else {
ConversionState.Converting(input, 0)
}
WorkInfo.State.SUCCEEDED -> {
val path = info.outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)
if (path == null) {
ConversionState.Failed("Conversion reported success but produced no file.")
} else {
val staged = File(path)
// Take responsibility for the file at the same moment the state
// starts referring to it, so the two cannot disagree.
pendingStaged = staged
ConversionState.Converted(
input = input,
staged = staged,
engineUsed = info.outputData
.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
routeReason = info.outputData
.getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
suggestedName = info.outputData
.getString(ConversionWorker.KEY_SUGGESTED_NAME)
?.takeIf { it.isNotBlank() }
// Work enqueued before the worker reported this carries
// nothing, and WorkManager keeps finished work for about a
// week -- so this branch is ordinary for a few days rather
// than a corner. It is the old derivation, kept because it is
// the same guess the app already made and there is genuinely
// nothing better available for such a job. New work never
// reaches it.
?: ConversionWorker.outputNameFor(
input.displayName,
_settings.value.spec,
),
mimeType = info.outputData
.getString(ConversionWorker.KEY_MIME_TYPE)
?.takeIf { it.isNotBlank() }
?: _settings.value.spec.mimeType,
)
}
}
// A worker that dies before it can report anything leaves no output data at
// all — a foreground-service start refused after a process restart is one
// way — and an exception's message can be an empty string. Both would read
// as a failure with nothing said, so blank falls back like missing does.
WorkInfo.State.FAILED -> ConversionState.Failed(
info.outputData.getString(ConversionWorker.KEY_ERROR)
?.takeIf { it.isNotBlank() }
?: "Conversion failed.",
)
WorkInfo.State.CANCELLED -> cancelled
WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
}
val next = conversionStateFrom(
ConversionUpdate(
state = info.state,
progressPercent = info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
runAttemptCount = info.runAttemptCount,
outputData = info.outputData,
),
input = input,
cancelled = cancelled,
fallbackSpec = _settings.value.spec,
)
// Take responsibility for the file at the same moment the state starts referring
// to it, so the two cannot disagree. Read off the result rather than assigned
// inside the mapping: `Converted` is the only state that carries a staged file, so
// "the state and `pendingStaged` refer to the same file or to no file" is now the
// shape of the code rather than a rule two branches have to keep.
if (next is ConversionState.Converted) pendingStaged = next.staged
_state.value = next
}
}
}
@@ -565,7 +628,7 @@ class ConversionViewModel @JvmOverloads constructor(
// than a fresh handle for the second failure's sake: a retry that fails again
// lands back here still carrying the file, not on a bare Failed that would take
// the offer away.
_state.value = ConversionState.Failed(e.message ?: "Could not save the file.", pending)
_state.value = ConversionState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)
}
}
}
@@ -94,24 +94,61 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
state = state,
settings = settings,
validation = validation,
actions = ConverterActions(
actions = converterActions(
viewModel = viewModel,
onPickInput = { pickInput.launch(arrayOf("*/*")) },
onPreset = viewModel::setPreset,
onContainer = viewModel::setContainer,
onVideoCodec = viewModel::setVideoCodec,
onAudioCodec = viewModel::setAudioCodec,
onSuggestion = viewModel::applySuggestion,
onQuality = viewModel::setQuality,
onEnginePreference = viewModel::setEnginePreference,
onConvert = { requestNotifications.launch(Manifest.permission.POST_NOTIFICATIONS) },
onCancel = viewModel::cancel,
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
onReset = viewModel::reset,
),
modifier = modifier,
)
}
/**
* Which of the ViewModel's methods each affordance on the screen calls.
*
* ## Why this is a function rather than an argument list
*
* It was an argument list, inside [ConverterScreen], which no test reached: `ConverterScreenContent`
* builds its own [ConverterActions], so every test in the suite drove the stateless inner and none
* of them ever saw the wiring.
*
* Most of the list is safe without a test, and saying so is more useful than pretending otherwise:
* `onContainer`, `onVideoCodec`, `onAudioCodec`, `onPreset`, `onSuggestion`, `onQuality` and
* `onEnginePreference` each take a distinct type, so binding one to another's setter does not
* compile. Verified rather than assumed — swapping `onVideoCodec` and `onAudioCodec` fails with
* *"Inapplicable candidate(s): fun setAudioCodec(codec: AudioCodec)"*.
*
* **[ConverterActions.onCancel] and [ConverterActions.onReset] are the exception.** Both are
* `() -> Unit`, so swapping them compiles silently — also verified — and ships a Cancel button that
* throws the conversion away and a Start-over button that leaves it on screen. That pair is what
* `ConverterWiringTest` exists for.
*
* The three launcher-backed actions stay parameters: they need an `ActivityResultLauncher`, which
* is the part that genuinely needs the composition, and keeping them out means the rest can be
* checked without one.
*/
@UnstableApi
internal fun converterActions(
viewModel: ConversionViewModel,
onPickInput: () -> Unit,
onConvert: () -> Unit,
onSave: (suggestedName: String) -> Unit,
): ConverterActions = ConverterActions(
onPickInput = onPickInput,
onPreset = viewModel::setPreset,
onContainer = viewModel::setContainer,
onVideoCodec = viewModel::setVideoCodec,
onAudioCodec = viewModel::setAudioCodec,
onSuggestion = viewModel::applySuggestion,
onQuality = viewModel::setQuality,
onEnginePreference = viewModel::setEnginePreference,
onConvert = onConvert,
onCancel = viewModel::cancel,
onSave = onSave,
onReset = viewModel::reset,
)
/**
* Everything [ConverterScreenContent] can ask for, in one value.
*
@@ -92,7 +92,12 @@ object MediaProbe {
else -> InputKind.UNPARSEABLE
}
private class Extracted(
/**
* `internal` rather than `private` so [extractedFrom] can be named from a test. The JVM test
* source set is a friend of `main`, so this stays invisible outside the module — the precedent
* is `MainActivity`'s `Destination`, and [containerFrom] beside it.
*/
internal class Extracted(
val videoCodec: String?,
val audioCodec: String?,
val durationMs: Long,
@@ -100,33 +105,55 @@ object MediaProbe {
val height: Int,
)
/**
* What a set of track formats says about a file.
*
* Split out of [probeWithExtractor] so the rules below can be tested against tracks a test
* *chooses*, rather than against whatever the committed fixtures happen to contain. The device
* tests exercise this through real files; none of them can construct a two-video-track input,
* a track that omits its duration, or an audio-before-video ordering on purpose.
*
* Three rules live here, and each is a decision rather than plumbing:
*
* - **First track of a type wins.** `video == null` is the whole guard. A file with two video
* tracks must report the first, because that is the one an engine will transcode.
* - **Duration is the maximum across tracks**, not the first one found or the last. A file
* whose audio outlasts its video is ordinary, and reporting the video's length would cut the
* progress bar short.
* - **A track that omits `KEY_DURATION` contributes nothing** rather than zero. `MediaExtractor`
* omits it for plenty of real tracks — see `MediaProbeTrackFieldsTest` — and `maxOf` against a
* fabricated 0 would still be correct here, but reading a key that is absent is not.
*/
internal fun extractedFrom(formats: List<MediaFormat>): Extracted {
var video: String? = null
var audio: String? = null
var durationUs = 0L
var width = 0
var height = 0
for (format in formats) {
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
if (format.containsKey(MediaFormat.KEY_DURATION)) {
durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION))
}
when {
mime.startsWith("video/") && video == null -> {
video = shortName(mime)
width = format.intOr(MediaFormat.KEY_WIDTH)
height = format.intOr(MediaFormat.KEY_HEIGHT)
}
mime.startsWith("audio/") && audio == null -> audio = shortName(mime)
}
}
return Extracted(video, audio, durationUs / US_PER_MS, width, height)
}
private fun probeWithExtractor(context: Context, uri: Uri): Extracted? {
val extractor = MediaExtractor()
return try {
extractor.setDataSource(context, uri, null)
var video: String? = null
var audio: String? = null
var durationUs = 0L
var width = 0
var height = 0
for (i in 0 until extractor.trackCount) {
val format = extractor.getTrackFormat(i)
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
if (format.containsKey(MediaFormat.KEY_DURATION)) {
durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION))
}
when {
mime.startsWith("video/") && video == null -> {
video = shortName(mime)
width = format.intOr(MediaFormat.KEY_WIDTH)
height = format.intOr(MediaFormat.KEY_HEIGHT)
}
mime.startsWith("audio/") && audio == null -> audio = shortName(mime)
}
}
Extracted(video, audio, durationUs / US_PER_MS, width, height)
extractedFrom(extractor.trackFormats())
} catch (e: Exception) {
Log.i(TAG, "Platform extractor could not read $uri.", e)
null
@@ -269,25 +296,7 @@ object MediaProbe {
val extractor = MediaExtractor()
return try {
extractor.setDataSource(context, uri, null)
var video: String? = null
var audio: String? = null
var width = 0
var height = 0
var fps = 0
for (i in 0 until extractor.trackCount) {
val format = extractor.getTrackFormat(i)
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
if (mime.startsWith("video/") && video == null) {
video = shortName(mime)
width = format.intOr(MediaFormat.KEY_WIDTH)
height = format.intOr(MediaFormat.KEY_HEIGHT)
fps = format.intOr(MediaFormat.KEY_FRAME_RATE)
} else if (mime.startsWith("audio/") && audio == null) {
audio = shortName(mime)
}
}
ConcatInput(video, audio, width, height, fps)
concatInputFrom(extractor.trackFormats())
} catch (e: Exception) {
Log.i(TAG, "Could not probe $uri for concat; will re-encode.", e)
ConcatInput(null, null, 0, 0, 0)
@@ -296,6 +305,45 @@ object MediaProbe {
}
}
/**
* The join flow's read of the same track formats. See [extractedFrom] for why this is separate
* from the extractor.
*
* Deliberately **not** folded into [extractedFrom] despite the overlap. This one reads frame
* rate and does not read duration; that one reads duration and does not read frame rate. A
* merged version would have to compute both for every caller, and `ConcatPlanner` treats an
* unknown frame rate as "cannot prove a match" — so a field this flow does not need must not
* start arriving as a number.
*/
internal fun concatInputFrom(formats: List<MediaFormat>): ConcatInput {
var video: String? = null
var audio: String? = null
var width = 0
var height = 0
var fps = 0
for (format in formats) {
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
if (mime.startsWith("video/") && video == null) {
video = shortName(mime)
width = format.intOr(MediaFormat.KEY_WIDTH)
height = format.intOr(MediaFormat.KEY_HEIGHT)
fps = format.intOr(MediaFormat.KEY_FRAME_RATE)
} else if (mime.startsWith("audio/") && audio == null) {
audio = shortName(mime)
}
}
return ConcatInput(video, audio, width, height, fps)
}
/**
* Every track format this extractor holds, read once.
*
* The thin edge the two pure functions above leave behind: a `trackCount` and a
* `getTrackFormat` per index, which is the whole of what needs a real `MediaExtractor`.
*/
private fun MediaExtractor.trackFormats(): List<MediaFormat> = (0 until trackCount).map(::getTrackFormat)
/**
* One track property as an Int, or [fallback] when the format has no Int to give.
*
@@ -5,6 +5,7 @@ import android.net.Uri
import android.provider.DocumentsContract
import android.provider.OpenableColumns
import java.io.File
import java.io.OutputStream
/**
* What a save has to say when the staged file is not there any more.
@@ -23,6 +24,20 @@ const val STAGED_FILE_GONE_MESSAGE: String =
"The finished file is no longer in the cache, so there is nothing left to save. " +
"Start over to make it again."
/**
* What to tell the user when the copy into their chosen destination did not finish.
*
* A fallback, not the usual message: `publish` throws with a real reason most of the time — the
* volume filled, the provider revoked the grant — and that reason is better than this. This is for
* the exception that arrives with nothing to say, which would otherwise reach the screen as an
* empty failure.
*
* Kept next to [STAGED_FILE_GONE_MESSAGE] for exactly the reason that one names: **both ViewModels
* need it**, and saving is what it is about. It was written out twice before — `ConversionViewModel`
* and `JoinViewModel` each carried their own copy of the literal, agreeing by coincidence.
*/
const val SAVE_FAILED_MESSAGE: String = "Could not save the file."
/**
* A staged file that is still there to be saved, and everything the save dialog needs to offer it.
*
@@ -170,7 +185,7 @@ open class OutputPublisher(private val context: Context) {
open fun publish(staged: File, destination: Uri) {
val destinationWasEmpty = destinationIsKnownEmpty(destination)
try {
val out = context.contentResolver.openOutputStream(destination)
val out = openDestination(destination)
?: error("Could not open destination for writing: $destination")
out.use { sink -> staged.inputStream().use { source -> source.copyTo(sink) } }
} catch (failure: Throwable) {
@@ -179,6 +194,22 @@ open class OutputPublisher(private val context: Context) {
}
}
/**
* Opens [destination] for writing, or null when the provider will not.
*
* A seam, and a narrow one: it exists because `openOutputStream` has **two** ways of refusing
* and only one of them is reachable from a test otherwise. A provider that has gone away throws
* `FileNotFoundException` from inside the call; a provider that is present and declines returns
* null. The two are not interchangeable here — the `?: error(...)` above is the only thing that
* turns the second into a failure rather than an NPE further down — and no fake provider can be
* asked to produce a null return on demand.
*
* `protected open` rather than injected, matching `hasSpaceFor` and `createStagingFile`:
* `WorkerStubs.kt`'s publishers already override one method to force one condition.
*/
protected open fun openDestination(destination: Uri): OutputStream? =
context.contentResolver.openOutputStream(destination)
/**
* True only when the destination is *positively known* to hold no bytes yet.
*
@@ -256,7 +287,7 @@ open class OutputPublisher(private val context: Context) {
open fun sweepStaging(nowMs: Long = System.currentTimeMillis()) {
val dir = stagingDir
val listing = dir.listFiles() ?: return
val entries = listing.map { StagingSweep.Entry(it.name, it.lastModified()) }
val entries = snapshot(listing)
StagingSweep.collectable(entries, nowMs).forEach { name ->
val file = File(dir, name)
// Re-read the timestamp rather than trusting the snapshot above. Between the
@@ -268,6 +299,22 @@ open class OutputPublisher(private val context: Context) {
}
}
/**
* The name and age of everything [sweepStaging] found, read once.
*
* A seam for the *race*, not for the clock — [sweepStaging] already takes `nowMs`, so the clock
* is the caller's. What has no seam otherwise is the window between this snapshot and the
* per-file re-read below it, and that window is the entire reason the re-read exists.
*
* **It has to be here and not around `listFiles()`.** A test that changes a file before the
* listing, or during it, changes what `StagingSweep.collectable` is given — so the file is
* never proposed for deletion and the re-read is never reached. The race being modelled is a
* file that *was* collectable when the snapshot was taken and is not by the time the delete
* comes round, which is exactly one worker resuming in this same process.
*/
protected open fun snapshot(listing: Array<File>): List<StagingSweep.Entry> =
listing.map { StagingSweep.Entry(it.name, it.lastModified()) }
private fun File.canonicalOrAbsolute(): File = runCatching { canonicalFile }.getOrDefault(absoluteFile)
private companion object {
@@ -56,17 +56,38 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
JoinScreenContent(
state = state,
actions = JoinActions(
actions = joinActions(
viewModel = viewModel,
onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
onJoin = viewModel::join,
onCancel = viewModel::cancel,
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
onReset = viewModel::reset,
),
modifier = modifier,
)
}
/**
* Which of the ViewModel's methods each affordance on the join screen calls.
*
* The join-side twin of `converterActions`, and the transposition risk here is worse: **three**
* `() -> Unit` bindings rather than two. `onJoin`, `onCancel` and `onReset` are mutually
* interchangeable as far as the compiler is concerned, so a Join button that cancels, or a Cancel
* button that starts the job, is a swap nothing but a test would catch.
*
* See `converterActions` for why the launcher-backed actions stay parameters.
*/
@UnstableApi
internal fun joinActions(
viewModel: JoinViewModel,
onPickInputs: () -> Unit,
onSave: (suggestedName: String) -> Unit,
): JoinActions = JoinActions(
onPickInputs = onPickInputs,
onJoin = viewModel::join,
onCancel = viewModel::cancel,
onSave = onSave,
onReset = viewModel::reset,
)
/**
* Everything [JoinScreenContent] can ask for, in one value.
*
@@ -5,6 +5,7 @@ import android.net.Uri
import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.viewModelScope
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.WorkInfo
import androidx.work.WorkManager
import kotlinx.coroutines.CoroutineDispatcher
@@ -19,6 +20,7 @@ import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.InputFile
import org.libremediaconverter.convert.InputQuery
import org.libremediaconverter.convert.PendingSave
import org.libremediaconverter.convert.SAVE_FAILED_MESSAGE
import org.libremediaconverter.convert.STAGED_FILE_GONE_MESSAGE
import org.libremediaconverter.convert.ScreenOwnership
import org.libremediaconverter.model.ConcatStrategy
@@ -29,6 +31,108 @@ import org.libremediaconverter.work.jobSnapshots
import java.io.File
import java.util.UUID
/**
* One update about a running join, as WorkManager last reported it.
*
* The join-side twin of `ConversionUpdate`, and deliberately the same shape: this pair of seams is
* one refactor done twice, and letting them diverge would make the two flows harder to compare than
* the duplication costs. There is no `progressPercent` here because `ConcatWorker` publishes none —
* a join is indeterminate.
*/
internal data class JoinUpdate(val state: WorkInfo.State, val runAttemptCount: Int, val outputData: Data)
/**
* What the join screen should show, given what WorkManager last said about the job.
*
* The join-side twin of `conversionStateFrom`, extracted for the same reason and with the same two
* exclusions: the ownership check stays at the call site, and this takes no responsibility for the
* staged file. See that function's KDoc for the argument in full.
*
* Five arms had never been chosen by any test before this was cut out, because a real `ConcatWorker`
* only ever produces a terminal state with well-formed output.
*
* @param cancelled where a cancellation lands, which differs for a reattached job — see [observe].
*/
@UnstableApi
internal fun joinStateFrom(update: JoinUpdate, inputs: List<InputFile>, cancelled: JoinState): JoinState =
when (update.state) {
// BLOCKED is a job waiting on a prerequisite, which the user has nothing to do about and
// nothing useful to be told about. It reads as "starting", like a fresh ENQUEUED.
WorkInfo.State.RUNNING, WorkInfo.State.BLOCKED -> JoinState.Joining(inputs)
// ENQUEUED after a run means a retry is pending -- the same rule, and the same reasoning, as
// the convert side. See `conversionStateFrom`.
WorkInfo.State.ENQUEUED ->
if (update.runAttemptCount > 0) {
JoinState.Waiting(inputs)
} else {
JoinState.Joining(inputs)
}
WorkInfo.State.SUCCEEDED -> joinedFrom(update.outputData)
// A worker that dies before it can report anything leaves no output data at all, and an
// exception's message can be an empty string. Both would read as a failure with nothing said,
// so blank falls back like missing does.
WorkInfo.State.FAILED -> JoinState.Failed(
update.outputData.getString(ConcatWorker.KEY_ERROR)
?.takeIf { it.isNotBlank() }
?: ConcatWorker.GENERIC_FAILURE_MESSAGE,
)
WorkInfo.State.CANCELLED -> cancelled
}
/**
* The `SUCCEEDED` arm. A join that reported success and named no file has nothing to offer.
*/
@UnstableApi
private fun joinedFrom(outputData: Data): JoinState {
val path = outputData.getString(ConcatWorker.KEY_OUTPUT_PATH)
?: return JoinState.Failed(JOINED_WITHOUT_A_FILE_MESSAGE)
return JoinState.Joined(
staged = File(path),
strategy = strategyFrom(outputData.getString(ConcatWorker.KEY_STRATEGY)),
// A join enqueued before the worker reported these carries neither, and the fallback is
// the format such a job really used -- ConcatWorker.request has always defaulted to it,
// and the join screen has never offered a choice.
suggestedName = outputData.getString(ConcatWorker.KEY_SUGGESTED_NAME)
?.takeIf { it.isNotBlank() }
?: ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT),
mimeType = outputData.getString(ConcatWorker.KEY_MIME_TYPE)
?.takeIf { it.isNotBlank() }
?: ConcatWorker.DEFAULT_FORMAT.mimeType,
)
}
/**
* The strategy a finished join reported, or [ConcatStrategy.REENCODE] when it named none.
*
* **Looked up rather than `valueOf`, and that is a fix rather than a style choice.** `valueOf`
* throws `IllegalArgumentException` on a name this build does not define, and this runs inside a
* `viewModelScope` collect with no handler -- so the throw does not become a `Failed` state, it
* takes the process down.
*
* Reachable for the reason `WorkerEnumFallbackTest` and `JobTags` are both written on: WorkManager
* keeps finished work for about a week, so a downgrade or a rollback hands this build a job
* enqueued by another one. `ConcatWorker` writes `result.strategy.name` into the output `Data`, so
* a build that added a third strategy would leave this one crashing on its own completed joins.
*
* `ConcatWorker.kt` already made exactly this change for `KEY_FORMAT`, and says why in as many
* words: *"Looked up rather than `valueOf` … a format name this build does not define used to throw
* past the catch."* The same read on this side had not been changed with it.
*
* REENCODE is the safe default rather than an arbitrary one: it is the answer for inputs that do
* not match, so a job whose strategy cannot be read is described as the more conservative of the
* two rather than being claimed as a lossless stream copy.
*/
private fun strategyFrom(name: String?): ConcatStrategy =
ConcatStrategy.entries.firstOrNull { it.name == name } ?: ConcatStrategy.REENCODE
/** A join that reported success and named no file. There is nothing to offer the user to save. */
internal const val JOINED_WITHOUT_A_FILE_MESSAGE: String =
"Joining reported success but produced no file."
sealed interface JoinState {
data object Idle : JoinState
data class Ready(val inputs: List<InputFile>) : JoinState
@@ -194,7 +298,7 @@ class JoinViewModel @JvmOverloads constructor(
fun onInputsPicked(uris: List<Uri>) {
val token = ownership.claim()
if (uris.size < 2) {
_state.value = JoinState.Failed("Pick at least two files to join.")
_state.value = JoinState.Failed(ConcatWorker.TOO_FEW_INPUTS_MESSAGE)
return
}
viewModelScope.launch {
@@ -250,56 +354,19 @@ class JoinViewModel @JvmOverloads constructor(
// takes ownership of the staged file, and a superseded observation must not do
// that either.
if (!ownership.stillHeldBy(token)) return@collect
_state.value = when (info.state) {
WorkInfo.State.RUNNING, WorkInfo.State.BLOCKED -> JoinState.Joining(inputs)
WorkInfo.State.ENQUEUED ->
if (info.runAttemptCount > 0) {
JoinState.Waiting(inputs)
} else {
JoinState.Joining(inputs)
}
WorkInfo.State.SUCCEEDED -> {
val path = info.outputData.getString(ConcatWorker.KEY_OUTPUT_PATH)
val strategy = info.outputData.getString(ConcatWorker.KEY_STRATEGY)
?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE
if (path == null) {
JoinState.Failed("Joining reported success but produced no file.")
} else {
val staged = File(path)
// Take responsibility for the file at the same moment the state
// starts referring to it, so the two cannot disagree.
pendingStaged = staged
JoinState.Joined(
staged = staged,
strategy = strategy,
// A join enqueued before the worker reported these carries
// neither, and the fallback is the format such a job really
// used -- ConcatWorker.request has always defaulted to it, and
// the join screen has never offered a choice.
suggestedName = info.outputData
.getString(ConcatWorker.KEY_SUGGESTED_NAME)
?.takeIf { it.isNotBlank() }
?: ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT),
mimeType = info.outputData
.getString(ConcatWorker.KEY_MIME_TYPE)
?.takeIf { it.isNotBlank() }
?: ConcatWorker.DEFAULT_FORMAT.mimeType,
)
}
}
// A worker that dies before it can report anything leaves no output data at
// all, and an exception's message can be an empty string. Both would read as
// a failure with nothing said, so blank falls back like missing does.
WorkInfo.State.FAILED -> JoinState.Failed(
info.outputData.getString(ConcatWorker.KEY_ERROR)
?.takeIf { it.isNotBlank() }
?: "Joining failed.",
)
WorkInfo.State.CANCELLED -> cancelled
}
val next = joinStateFrom(
JoinUpdate(
state = info.state,
runAttemptCount = info.runAttemptCount,
outputData = info.outputData,
),
inputs = inputs,
cancelled = cancelled,
)
// Read off the result rather than assigned inside the mapping -- see the same
// three lines in ConversionViewModel for why that is the better half of the swap.
if (next is JoinState.Joined) pendingStaged = next.staged
_state.value = next
}
}
}
@@ -346,7 +413,7 @@ class JoinViewModel @JvmOverloads constructor(
// than leaving "Start over" -- which deletes it -- as the only thing on offer.
// Passing `pending` rather than rebuilding it is what keeps a retry that fails
// again on a carrying Failed instead of a bare one.
_state.value = JoinState.Failed(e.message ?: "Could not save the file.", pending)
_state.value = JoinState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)
}
}
}
@@ -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"
@@ -39,7 +39,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse)
?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
if (uris.size < 2) {
return Result.failure(workDataOf(KEY_ERROR to "Pick at least two files to join."))
return Result.failure(workDataOf(KEY_ERROR to TOO_FEW_INPUTS_MESSAGE))
}
// Absent, not zero, when the picker could not size every input -- see the same read in
// ConversionWorker and InputQuery for why the two are no longer one number.
@@ -107,7 +107,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
}
FailureOutcome.FAIL -> {
Log.e(TAG, "Joining failed.", e)
Result.failure(workDataOf(KEY_ERROR to (e.message ?: "Joining failed.")))
Result.failure(workDataOf(KEY_ERROR to (e.message ?: GENERIC_FAILURE_MESSAGE)))
}
}
}
@@ -137,6 +137,34 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
)
companion object {
/**
* What the user is told when a join arrives with fewer than two inputs.
*
* Shared with `JoinViewModel`, which refuses the same condition one layer up so the picker
* can answer without enqueueing anything. Two copies of this sentence existed before, and
* only the one here was pinned by a test (#139) — so the wording could drift on the screen
* without a single test noticing, for one message the user sees from one condition.
*
* 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.
*/
const val TOO_FEW_INPUTS_MESSAGE: String = "Pick at least two files to join."
/**
* The last resort when a join fails and the exception says nothing.
*
* Shared with `JoinViewModel`, whose `FAILED` arm falls back to the same sentence when the
* output `Data` carries no error at all — a worker killed before it could write one. The two
* are a chain rather than a coincidence: this is what the worker puts *in* `KEY_ERROR`, and
* that is what the ViewModel says when `KEY_ERROR` never arrived. The user cannot tell the
* two apart and should not have to, so they are one sentence.
*
* The `Log.e` above deliberately keeps its own literal. A log line has a different audience
* and carries the exception with it; coupling it to the user-facing wording would mean
* rewording the screen to change a log.
*/
const val GENERIC_FAILURE_MESSAGE: String = "Joining failed."
const val KEY_INPUT_URIS = "input_uris"
const val KEY_TOTAL_BYTES = "total_bytes"
const val KEY_FORMAT = "format"
@@ -313,7 +313,7 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
}
FailureOutcome.FAIL -> {
Log.e(TAG, "Conversion failed.", cause)
Result.failure(workDataOf(KEY_ERROR to (cause.message ?: "Conversion failed.")))
Result.failure(workDataOf(KEY_ERROR to (cause.message ?: GENERIC_FAILURE_MESSAGE)))
}
}
@@ -352,6 +352,20 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
)
companion object {
/**
* The last resort when a conversion fails and the exception says nothing.
*
* Shared with `ConversionViewModel`, whose `FAILED` arm falls back to the same sentence when
* the output `Data` carries no error — a worker killed before it could write one, which a
* refused foreground start after a process restart produces. The two are a chain rather than
* a coincidence: this is what goes *into* `KEY_ERROR`, and that is what is said when
* `KEY_ERROR` never arrived. The user cannot tell those apart and should not have to.
*
* See [ConcatWorker.GENERIC_FAILURE_MESSAGE] for the join-side twin, and the note there
* about why the neighbouring `Log.e` keeps its own literal.
*/
const val GENERIC_FAILURE_MESSAGE: String = "Conversion failed."
const val KEY_INPUT_URI = "input_uri"
const val KEY_DISPLAY_NAME = "display_name"
const val KEY_SIZE_BYTES = "size_bytes"
@@ -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) }
}
}
@@ -7,6 +7,7 @@ import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.libremediaconverter.model.CodecNames
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.VideoCodec
/**
@@ -133,6 +134,38 @@ class CodecVocabularyTest {
* landed, a device with no HEVC decoder answered true for `x265` and Media3 was handed a job it
* could not do; now the router sends it to FFmpeg without spending the attempt.
*/
/**
* The sentinel is not just another unknown name, and the difference is the whole guard.
*
* `canDecode` ends `?: true` -- a name neither table knows keeps the permissive answer, because
* the app would rather try than refuse a file it might handle. `InputProbe.UNPARSEABLE` has to
* be the exception: the platform has *already* failed to parse the input, so there is nothing
* for a decoder to be permissive about, and waving it through spends a Media3 attempt on a job
* that cannot start.
*
* The `cinepak` line is what makes the sentinel line mean something. Without it, deleting the
* early return leaves this test green -- both names would fall through to the same `?: true`.
* The pair is the assertion.
*
* `DeviceCodecs.PERMISSIVE` carries the same rule and `ConversionRouterTest` pins its routing
* consequence. This is the implementation that runs on a device.
*/
@Test
fun `the unparseable sentinel is refused even where an unknown name is waved through`() {
val everything = AndroidDeviceCodecs.forTesting(
encoders = emptySet(),
decoders = setOf("video/avc", "video/hevc"),
)
assertFalse(
"the platform could not parse this input, so there is nothing to decode with",
everything.canDecode(InputProbe.UNPARSEABLE),
)
assertTrue(
"a merely unknown name still keeps the permissive answer",
everything.canDecode("cinepak"),
)
}
@Test
fun `a device without the decoder now says so for the aliases it used to wave through`() {
val hevcOnly = AndroidDeviceCodecs.forTesting(encoders = emptySet(), decoders = setOf("video/hevc"))
@@ -0,0 +1,238 @@
package org.libremediaconverter.convert
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.WorkInfo
import androidx.work.workDataOf
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.work.ConversionWorker
import org.robolectric.RobolectricTestRunner
/**
* Every answer [conversionStateFrom] can give, chosen rather than stumbled into.
*
* ## What this revises
*
* The mapping is not cold code and never was: `ConversionViewModel$observe$1$1` reported 28 covered
* lines before this file existed, because every test that drives a real worker runs it. What no
* test did was **choose which arm it took**. A real worker reaches a terminal state with
* well-formed output, so `SUCCEEDED`-with-a-path and `FAILED`-with-a-message were the only arms any
* test had ever produced — the other six ran never.
*
* A `grep` for `WorkInfo.State.` across the JVM suite makes that look untrue: all six constants are
* there. They are in `ReattachmentTest`, driven into **`Reattachment.choose`** — a different
* function that encodes the same enqueued-means-retry rule. So that rule had a test in one of its
* two homes, and the copy the user's screen reads had none.
*
* ## Why the seam, and why these assertions
*
* `WorkManager.getInstance` is called in the ViewModel's constructor and `observe` is private, so
* nothing could hand this a chosen `WorkInfo`. Cutting the `when` out as a pure function over
* [ConversionUpdate] is the answer #141 took for `MediaProbe`, and `JobSnapshot` beside
* `Reattachment.choose` is the same shape again.
*
* The assertions are on the whole state, not on its type. `Converting(input, 40)` and
* `Converting(input, 0)` are both `Converting`, and a mapping that dropped the progress read would
* pass any test that only asked which class came back.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ConversionStateMappingTest {
// --- running ------------------------------------------------------------
@Test
fun `a running job reports the progress it published`() {
// The percent is read from `progress`, not from `outputData`, and not from the settings.
// A mapping that returned Converting(input, 0) for every RUNNING would leave the bar
// pinned at zero for the whole conversion.
val state = map(WorkInfo.State.RUNNING, progress = 40)
assertEquals(ConversionState.Converting(INPUT, 40), state)
}
@Test
fun `a running job with no published progress reports zero rather than failing`() {
// getInt's default. A worker that has started but not yet called setProgress is ordinary,
// and must not read as an error.
val state = map(WorkInfo.State.RUNNING, progress = null)
assertEquals(ConversionState.Converting(INPUT, 0), state)
}
// --- enqueued: the rule that had a test only in its other home ----------
@Test
fun `an enqueued job that has already run is waiting to retry`() {
val state = map(WorkInfo.State.ENQUEUED, runAttemptCount = 1)
assertEquals(
"an ENQUEUED after a run is a pending retry, which the user is told about",
ConversionState.Waiting(INPUT),
state,
)
}
@Test
fun `an enqueued job that has never run is simply starting`() {
// The other side, and the reason the test above is not enough on its own: a mapping that
// ignored runAttemptCount and always answered Waiting would pass that one and fail this.
val state = map(WorkInfo.State.ENQUEUED, runAttemptCount = 0)
assertEquals(ConversionState.Converting(INPUT, 0), state)
}
// --- succeeded ----------------------------------------------------------
@Test
fun `a success that named no file is a failure, not an empty success`() {
// The job said it finished and named nothing. There is no file to offer, so `Converted`
// would put a Save button over a path that does not exist.
val state = map(WorkInfo.State.SUCCEEDED, data = Data.EMPTY)
assertEquals(ConversionState.Failed(SUCCEEDED_WITHOUT_A_FILE_MESSAGE), state)
}
@Test
fun `a success carries the worker's own name and type, not the current settings`() {
val state = map(
WorkInfo.State.SUCCEEDED,
data = workDataOf(
ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mkv",
ConversionWorker.KEY_SUGGESTED_NAME to "holiday.mkv",
ConversionWorker.KEY_MIME_TYPE to "video/x-matroska",
ConversionWorker.KEY_ENGINE_USED to "FFMPEG",
ConversionWorker.KEY_ROUTE_REASON to "container needs FFmpeg",
),
)
val converted = state as ConversionState.Converted
assertEquals("holiday.mkv", converted.suggestedName)
assertEquals("video/x-matroska", converted.mimeType)
assertEquals("FFMPEG", converted.engineUsed)
assertEquals("container needs FFmpeg", converted.routeReason)
}
@Test
fun `a success from older work falls back to the current settings for name and type`() {
// WorkManager keeps finished work about a week, so a job enqueued before the worker
// reported these is ordinary for a few days rather than a corner case.
val state = map(
WorkInfo.State.SUCCEEDED,
data = workDataOf(ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mp4"),
)
val converted = state as ConversionState.Converted
assertEquals(FALLBACK_SPEC.mimeType, converted.mimeType)
assertEquals(
ConversionWorker.outputNameFor(INPUT.displayName, FALLBACK_SPEC),
converted.suggestedName,
)
}
@Test
fun `a blank name or type falls back the same way a missing one does`() {
// A blank string is not an answer. Without takeIf, the save dialog opens named "" and
// registered for a MIME type of "", which no provider will accept.
val state = map(
WorkInfo.State.SUCCEEDED,
data = workDataOf(
ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mp4",
ConversionWorker.KEY_SUGGESTED_NAME to "",
ConversionWorker.KEY_MIME_TYPE to " ",
),
)
val converted = state as ConversionState.Converted
assertEquals(FALLBACK_SPEC.mimeType, converted.mimeType)
assertEquals(
ConversionWorker.outputNameFor(INPUT.displayName, FALLBACK_SPEC),
converted.suggestedName,
)
}
// --- failed -------------------------------------------------------------
@Test
fun `a failure carries the reason the worker gave`() {
val state = map(
WorkInfo.State.FAILED,
data = workDataOf(ConversionWorker.KEY_ERROR to "Not enough free space to convert."),
)
assertEquals(ConversionState.Failed("Not enough free space to convert."), state)
}
@Test
fun `a failure with nothing said still says something`() {
// A worker killed before it could write output data leaves none at all -- a refused
// foreground start after a process restart is one way. Failed("") would render as a blank
// error card.
val state = map(WorkInfo.State.FAILED, data = Data.EMPTY)
assertEquals(ConversionState.Failed(ConversionWorker.GENERIC_FAILURE_MESSAGE), state)
}
@Test
fun `a failure whose message is blank falls back like a missing one`() {
val state = map(
WorkInfo.State.FAILED,
data = workDataOf(ConversionWorker.KEY_ERROR to " "),
)
assertEquals(ConversionState.Failed(ConversionWorker.GENERIC_FAILURE_MESSAGE), state)
}
// --- cancelled and blocked ---------------------------------------------
@Test
fun `a cancellation lands wherever the caller said it should`() {
// Not a fixed state: a conversion started here goes back to Ready with the picked file,
// while one picked up by reattach goes to Idle, because the URI that job holds belongs to
// a process that no longer exists. `observe`'s KDoc is where that distinction is set.
val toReady = map(WorkInfo.State.CANCELLED, cancelled = ConversionState.Ready(INPUT))
val toIdle = map(WorkInfo.State.CANCELLED, cancelled = ConversionState.Idle)
assertEquals(ConversionState.Ready(INPUT), toReady)
assertEquals(ConversionState.Idle, toIdle)
}
@Test
fun `a blocked job looks like one that is starting`() {
// BLOCKED is a job waiting on a prerequisite. There is nothing useful to say about it that
// differs from "starting", and inventing a state for it would put a word on screen the
// user cannot act on.
val state = map(WorkInfo.State.BLOCKED)
assertEquals(ConversionState.Converting(INPUT, 0), state)
}
private fun map(
state: WorkInfo.State,
progress: Int? = null,
runAttemptCount: Int = 0,
data: Data = Data.EMPTY,
cancelled: ConversionState = ConversionState.Ready(INPUT),
): ConversionState = conversionStateFrom(
ConversionUpdate(
state = state,
// Modelled on the call site, which reads `getInt(KEY_PROGRESS, 0)` -- so "no progress
// published" is the default reaching the mapping, not a null it has to handle.
progressPercent = progress ?: 0,
runAttemptCount = runAttemptCount,
outputData = data,
),
input = INPUT,
cancelled = cancelled,
fallbackSpec = FALLBACK_SPEC,
)
private companion object {
val INPUT = InputFile(Uri.parse("content://test/holiday.mov"), "holiday.mov", 4096L)
val FALLBACK_SPEC = OutputFormat.MP4_H265.spec
}
}
@@ -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,221 @@
package org.libremediaconverter.convert
import android.media.MediaFormat
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
/**
* The rules `MediaProbe` applies to a set of track formats.
*
* ## Why this exists, and what it revises
*
* Issue #84 classified `probeWithExtractor` and `probeForConcat` as device-bound and explicitly not
* a gap:
*
* > These are exercised by `RemuxTest`, `ConcatEngineTest` and `RealMediaBenchmark` in
* > `androidTest` … **Do not read their 0% as untested.**
*
* That was right about the measurement boundary and right about FFprobe. It was not right that
* these are only orchestration. The track walk is a **branch matrix**, and `androidTest` reaches it
* only through whatever the committed fixtures happen to contain — so none of the rules below is
* *chosen* by any test there. A fixture with two video tracks, a track that omits its duration, or
* an audio-before-video ordering is not something a device test would produce on purpose.
*
* The seam is the answer #133 preferred over driving `ShadowMediaExtractor`: the walk is a pure
* function over `List<MediaFormat>`, and what is left needing a device — `setDataSource`,
* `getTrackFormat`, `release` — is the thin edge `androidTest` should be covering. This is the
* `work/FailureOutcome.kt` pattern `CLAUDE.md` names.
*
* `MediaFormat` is a real one throughout, not a stub. `MediaProbeTrackFieldsTest` records why that
* matters: it is a heterogeneous map whose getters throw rather than coerce, and a hand-rolled
* double would not reproduce that.
*/
@RunWith(RobolectricTestRunner::class)
class MediaProbeTrackWalkTest {
// --- extractedFrom: the conversion flow's read ---------------------------
@Test
fun `the first video track wins when a file carries two`() {
// `video == null` is the entire guard. A file with two video tracks must report the first,
// because that is the one an engine will transcode -- and the width and height must come
// from the same track, not be mixed across them.
val extracted = MediaProbe.extractedFrom(
listOf(
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080),
video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480),
),
)
assertEquals("h264", extracted.videoCodec)
assertEquals(1920, extracted.width)
assertEquals(1080, extracted.height)
}
@Test
fun `the first audio track wins when a file carries two`() {
val extracted = MediaProbe.extractedFrom(
listOf(
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
audio(MediaFormat.MIMETYPE_AUDIO_OPUS),
),
)
assertEquals("aac", extracted.audioCodec)
}
@Test
fun `duration is the longest track, not the first or the last`() {
// A file whose audio outlasts its video is ordinary. Taking the video's length would cut
// the progress bar short; taking the last track's would be right only by accident of order.
val extracted = MediaProbe.extractedFrom(
listOf(
video(MediaFormat.MIMETYPE_VIDEO_AVC, durationUs = 10_000_000),
audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 12_500_000),
audio(MediaFormat.MIMETYPE_AUDIO_OPUS, durationUs = 1_000_000),
),
)
assertEquals(12_500L, extracted.durationMs)
}
@Test
fun `a track that does not declare its duration contributes nothing to it`() {
// MediaExtractor omits KEY_DURATION for plenty of real tracks -- MediaProbeTrackFieldsTest
// records the same for KEY_FRAME_RATE. Reading a key that is absent is what containsKey
// stands between us and.
val extracted = MediaProbe.extractedFrom(
listOf(
video(MediaFormat.MIMETYPE_VIDEO_AVC),
audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 7_000_000),
),
)
assertEquals(7_000L, extracted.durationMs)
}
@Test
fun `declaring audio before video changes nothing`() {
// Track order is a property of the container, not of the content. Both orderings have to
// reach the same answer or the same file remuxed twice would probe differently.
val videoFirst = MediaProbe.extractedFrom(
listOf(
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720),
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
),
)
val audioFirst = MediaProbe.extractedFrom(
listOf(
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720),
),
)
assertEquals(videoFirst.videoCodec, audioFirst.videoCodec)
assertEquals(videoFirst.audioCodec, audioFirst.audioCodec)
assertEquals(videoFirst.width, audioFirst.width)
assertEquals(videoFirst.height, audioFirst.height)
}
@Test
fun `a track that is neither audio nor video is ignored`() {
// Subtitle and timed-metadata tracks are common in MKV and MP4. Neither prefix matches, so
// neither slot is filled -- and, importantly, a subtitle track must not be mistaken for the
// absence of an audio track by some later `else`.
val extracted = MediaProbe.extractedFrom(
listOf(
MediaFormat().apply { setString(MediaFormat.KEY_MIME, "text/vtt") },
video(MediaFormat.MIMETYPE_VIDEO_AVC),
),
)
assertEquals("h264", extracted.videoCodec)
assertNull(extracted.audioCodec)
}
@Test
fun `a file with no tracks reports nothing rather than zero-width video`() {
val extracted = MediaProbe.extractedFrom(emptyList())
assertNull(extracted.videoCodec)
assertNull(extracted.audioCodec)
assertEquals(0L, extracted.durationMs)
assertEquals(0, extracted.width)
assertEquals(0, extracted.height)
}
@Test
fun `an audio-only file reports no video codec at all`() {
// The distinction MediaProbe.classify turns into InputKind.AUDIO_ONLY, and the reason
// `hasVideo` exists: an audio file and a corrupt file must not look alike.
val extracted = MediaProbe.extractedFrom(listOf(audio(MediaFormat.MIMETYPE_AUDIO_AAC)))
assertNull(extracted.videoCodec)
assertEquals("aac", extracted.audioCodec)
assertEquals(0, extracted.width)
}
// --- concatInputFrom: the join flow's read -------------------------------
@Test
fun `the join read takes frame rate from the first video track`() {
val input = MediaProbe.concatInputFrom(
listOf(
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080, frameRate = 30),
video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480, frameRate = 60),
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
),
)
assertEquals("h264", input.videoCodec)
assertEquals("aac", input.audioCodec)
assertEquals(1920, input.width)
assertEquals(1080, input.height)
assertEquals(30, input.frameRate)
}
@Test
fun `a video track with no declared frame rate reports zero rather than guessing`() {
// ConcatPlanner treats 0 as "cannot prove a match" and re-encodes. A guessed 30 would read
// as agreement and produce a stream copy of clips that do not actually match -- the failure
// its KDoc says the whole flow is arranged to avoid.
val input = MediaProbe.concatInputFrom(listOf(video(MediaFormat.MIMETYPE_VIDEO_AVC)))
assertEquals(0, input.frameRate)
}
@Test
fun `a file with no tracks joins as entirely unknown`() {
val input = MediaProbe.concatInputFrom(emptyList())
assertNull(input.videoCodec)
assertNull(input.audioCodec)
assertEquals(0, input.width)
assertEquals(0, input.height)
assertEquals(0, input.frameRate)
}
private fun video(
mime: String,
width: Int = 1920,
height: Int = 1080,
durationUs: Long? = null,
frameRate: Int? = null,
): MediaFormat = MediaFormat.createVideoFormat(mime, width, height).apply {
durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) }
frameRate?.let { setInteger(MediaFormat.KEY_FRAME_RATE, it) }
}
private fun audio(mime: String, durationUs: Long? = null): MediaFormat =
MediaFormat.createAudioFormat(mime, SAMPLE_RATE, CHANNELS).apply {
durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) }
}
private companion object {
const val SAMPLE_RATE = 48_000
const val CHANNELS = 2
}
}
@@ -20,6 +20,9 @@ import java.io.OutputStream
/** What a destination volume says when it fills up mid-write. */
private const val NO_SPACE = "No space left on device"
/** How far a failing copy gets before the volume "fills up". Any value below the payload does. */
private const val PARTIAL_BYTES = 512
/**
* A sink that behaves like a volume filling up.
*
@@ -75,6 +78,8 @@ class OutputPublisherPublishTest {
private val payload = ByteArray(8192) { (it % 251).toByte() }
/** How far a failing copy gets before the volume "fills up". Any value below the payload does. */
private val documentUri: Uri = Uri.parse("content://$DOCUMENTS_AUTHORITY/document/holiday.mp4")
private val plainUri: Uri = Uri.parse("content://$PLAIN_AUTHORITY/document/holiday_plain.mp4")
private val deadUri: Uri = Uri.parse("content://org.libremediaconverter.nonexistent/document/gone.mp4")
@@ -203,6 +208,76 @@ class OutputPublisherPublishTest {
assertEquals(emptyList<Uri>(), FakeSafProvider.deleteRequests)
}
@Test
fun `a destination whose size cannot be determined is never deleted`() {
// The three short-circuits in destinationIsKnownEmpty, and the reason its KDoc gives for
// each of them answering false:
//
// "this decides whether a delete is allowed and 'I could not tell' must never authorise
// one."
//
// The contrast is `a copy that fails partway leaves nothing at the destination` above: a
// provider that *does* say zero gets the delete. These say nothing, so they must not.
// Getting this backwards costs the user a file they already had, on a save that failed.
//
// Named exemption: of the three conjuncts, `size >= 0` cannot be falsified behaviourally.
// Measured -- getColumnIndex returns -1 for an absent column, and isNull(-1) throws
// CursorIndexOutOfBoundsException, which the surrounding runCatching already turns into
// `?: false`. So relaxing it to `size >= -1` leaves this test green: same answer, reached
// by the exception path instead. The guard should stay -- control flow through an exception
// is worse than a comparison, and another Cursor implementation need not throw -- but no
// assertion here pins it, and saying so beats implying the missing-column case covers it.
// `!row.isNull(size)` and `row.moveToFirst()` do both bite.
listOf(
RowShape.NO_SIZE_COLUMN to "a cursor with no SIZE column",
RowShape.NULL_SIZE to "a cursor whose SIZE cell is null",
RowShape.NO_ROWS to "a cursor holding no rows",
// The third case the KDoc names -- "a resolver call that throws" -- and the one the
// list was missing. It reaches `?: false` through `runCatching` rather than through a
// cursor answer, so it is the only one of the four that proves the catch is load
// bearing: a provider that revokes its grant between the picker and the write must not
// have its document deleted on the way out.
RowShape.QUERY_THROWS to "a provider that throws out of query",
).forEach { (shape, description) ->
FakeSafProvider.deleteRequests.clear()
FakeSafProvider.backingFile(documentUri).writeBytes(ByteArray(0))
FakeSafProvider.rowShape = shape
failMidCopy(documentUri, afterBytes = PARTIAL_BYTES)
assertThrows(IOException::class.java) { publisher.publish(staged, documentUri) }
assertEquals(
"$description must not authorise a delete",
emptyList<Uri>(),
FakeSafProvider.deleteRequests,
)
assertTrue(
"$description must leave the destination where it was",
FakeSafProvider.backingFile(documentUri).exists(),
)
}
}
@Test
fun `a provider that declines by returning null fails with the destination named`() {
// openOutputStream has two ways of refusing, and only one of them is otherwise reachable.
// `a destination the provider will not open...` above drives the throwing one -- a provider
// that has gone away. This is the other: a provider that is present, answers, and hands
// back null. Without the `?: error(...)` that becomes an NPE inside `use`, which reaches
// the user as "Conversion failed." with a null message.
val nullOpening = object : OutputPublisher(context) {
override fun openDestination(destination: Uri): OutputStream? = null
}
val failure = runCatching { nullOpening.publish(staged, documentUri) }.exceptionOrNull()
assertTrue("a null stream must not appear to succeed, got $failure", failure != null)
assertTrue(
"the failure must name the destination rather than being a bare NPE; got ${failure?.message}",
failure?.message?.contains("Could not open destination for writing") == true,
)
}
@Test
fun `a copy that succeeds delivers every byte and deletes nothing`() {
shadowOf(context.contentResolver).registerOutputStreamSupplier(documentUri) {
@@ -1,6 +1,8 @@
package org.libremediaconverter.convert
import android.app.Application
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
@@ -26,14 +28,18 @@ import java.util.UUID
@RunWith(RobolectricTestRunner::class)
class OutputPublisherStagingTest {
private lateinit var app: Application
private lateinit var cacheDir: File
private lateinit var publisher: OutputPublisher
@Before
fun setUp() {
val context = RuntimeEnvironment.getApplication()
cacheDir = context.cacheDir
publisher = OutputPublisher(context)
// Held as a field rather than a local: the race test below builds an anonymous
// OutputPublisher, and inside that `object` expression a bare `context` resolves to the
// superclass's own constructor property, which is not initialised at the super call.
app = RuntimeEnvironment.getApplication()
cacheDir = app.cacheDir
publisher = OutputPublisher(app)
}
@Test
@@ -99,4 +105,103 @@ class OutputPublisherStagingTest {
publisher.sweepStaging()
}
@Test
fun `the sweep tolerates a staging path that is not a directory`() {
// The other half of `listFiles() ?: return`, and not the same as the case above: a missing
// directory is created by `stagingDir`'s own mkdirs() and lists as empty. Only a path that
// cannot be a directory makes listFiles() answer null, and a sweep that dereferenced that
// would take the app down on a launch rather than on a conversion -- AppStartSweepTest is
// where this runs from.
val stagingPath = stagingPathAsRegularFile()
publisher.sweepStaging()
assertTrue("the sweep must not have replaced the fixture", stagingPath.isFile)
}
/**
* 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
* caught and this machine does not reproduce.
*
* `LibreMediaConverterApp.onCreate` ends with
* `appScope.launch { OutputPublisher(...).sweepStaging() }` on `Dispatchers.IO`, and
* `sweepStaging` reads `stagingDir`, whose getter calls `mkdirs()`. Robolectric instantiates
* the application for every test that asks for one, so that background `mkdirs()` is in flight
* across the whole suite, on a thread the paused main looper does not control. Between deleting
* this path and writing it there is a window where the path does not exist and that `mkdirs()`
* can win, which is `FileNotFoundException: ... (Is a directory)` out of `writeBytes` -- run
* 33069641674 on #149, once, against 468 tests that pass here.
*
* Retrying closes it rather than narrowing it, because the race is not symmetric: `mkdirs()`
* fails on an existing regular file, so the invariant only has to survive being *established*.
* Once a write lands, nothing in the suite can turn this back into a directory.
*
* The wider problem -- application-scope IO work racing every Robolectric test that shares
* `cacheDir` -- is #159, and is deliberately not fixed here.
*/
private fun stagingPathAsRegularFile(): File {
val stagingPath = File(cacheDir, "conversions")
repeat(FIXTURE_ATTEMPTS) {
if (stagingPath.isFile) return stagingPath
stagingPath.deleteRecursively()
runCatching { stagingPath.writeBytes(ByteArray(FIXTURE_BYTES)) }
}
check(stagingPath.isFile) {
"the fixture needs $stagingPath to be a regular file and it is a directory; " +
"something recreated it $FIXTURE_ATTEMPTS times -- see #159"
}
return stagingPath
}
@Test
fun `a file that stops being collectable between the listing and the delete survives`() {
// The race the second timestamp read exists for, and the only branch of it that had never
// run. The comment in sweepStaging states the cost precisely: a worker resumed by
// WorkManager -- in this same process -- could have started writing this very file, and
// unlinking an inode a running job still holds open ends with the job reporting success for
// a path that no longer exists.
//
// So: a file old enough to collect at listing time, touched to now before the delete is
// reached. StagingSweep.collectable already said yes; isCollectable has to say no.
val orphan = publisher.createStagingFile(
StagingNames.forJob(UUID.randomUUID(), "mp4"),
).apply { writeBytes(ByteArray(4096)) }
assertTrue(orphan.setLastModified(System.currentTimeMillis() - StagingSweep.GRACE_PERIOD_MS - 60_000))
// Touched *after* the snapshot is taken, which is the only window that reaches the
// re-read. Doing it around listFiles() instead changes what StagingSweep.collectable is
// given, so the file is never proposed for deletion and the guard is never exercised --
// measured, and the reason the seam sits where it does.
val racing = object : OutputPublisher(app) {
override fun snapshot(listing: Array<File>): List<StagingSweep.Entry> =
super.snapshot(listing).also { orphan.setLastModified(System.currentTimeMillis()) }
}
racing.sweepStaging()
assertTrue(
"a file a live job started writing after the listing must not be unlinked",
orphan.exists(),
)
}
@Test
fun `discarding a file with no parent at all is refused`() {
// A relative name has no parent directory, so `staged.parentFile` is null. The handle
// reaches the ViewModel as a path string out of WorkInfo.outputData and is turned straight
// into a File, so this is not a shape the caller can rule out -- and the guard has to
// answer false rather than dereference it.
val parentless = File("holiday.mp4")
assertNull("the fixture is supposed to have no parent", parentless.parentFile)
assertFalse("a file with no parent is not in staging", publisher.discardStaged(parentless))
}
private companion object {
/** Enough to outlast a burst of application-scope sweeps; one attempt is what CI lost. */
const val FIXTURE_ATTEMPTS = 50
const val FIXTURE_BYTES = 8
}
}
@@ -0,0 +1,241 @@
package org.libremediaconverter.convert
import android.app.Application
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.workDataOf
import org.junit.After
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.join.JoinState
import org.libremediaconverter.join.JoinViewModel
import org.libremediaconverter.join.joinActions
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.QualityTier
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.work.ConcatWorker
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
/**
* That each affordance is wired to the ViewModel method it is named after.
*
* ## What this covers that no other test can
*
* `ConverterScreenContentTest`, `ConverterStateAffordancesTest` and `JoinScreenContentTest` all
* drive the **stateless** content composables, which build their own `ConverterActions`. So the
* wiring — the list of `viewModel::` references the stateful outer hands down — was seen by nothing
* in the suite.
*
* ## The hazard is narrower than "seventeen bindings", and this says so
*
* #156 was filed claiming a transposition of any two bindings would survive the suite. That is not
* true, and it was worth checking rather than testing on the assumption:
*
* | swap | result |
* |---|---|
* | `onVideoCodec` ↔ `onAudioCodec` | **rejected by the compiler** |
* | `onCancel` ↔ `onReset` | **compiles** |
*
* Every typed binding — container, both codecs, preset, suggestion, quality, engine preference —
* takes a distinct parameter type, so the compiler is already the test. Writing assertions for
* those would be theatre.
*
* **The `() -> Unit` bindings are the real gap**, because they are interchangeable to the compiler:
* two on the converter screen (`onCancel`, `onReset`) and three on the join screen (`onJoin`,
* `onCancel`, `onReset`). A Cancel that discards the finished file, or a Join that cancels, is a
* one-character mistake that ships.
*
* ## How they are told apart
*
* By effect, not by a recording double. `reset()` sets the state to `Idle`; `cancel()` with no
* active job leaves it alone (`ConversionViewModel.cancel` is `activeWorkId?.let(...)`, and
* `SettingsEditsTest` pins that). Driving each from a non-`Idle` state is therefore enough to say
* which one ran.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ScreenWiringTest {
private lateinit var app: Application
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
ConversionDependencies.publisher = { RecordingPublisher(app) }
ConversionDependencies.probe = { _, _ -> org.libremediaconverter.model.InputProbe() }
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
// --- the converter screen ----------------------------------------------
@Test
fun `Start over resets, and Cancel does not`() {
// The transposition that compiles. If onReset were bound to cancel, this stays on Ready.
installTestWorkManager(app, Data.EMPTY)
val pick = ParkedPickDispatcher()
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
viewModel.onInputPicked(INPUT_URI)
pick.runAll()
assertNotEquals(
"the fixture needs a non-Idle state or neither action is observable",
ConversionState.Idle,
viewModel.state.value,
)
actions.onReset()
assertEquals(ConversionState.Idle, viewModel.state.value)
}
@Test
fun `Cancel leaves the picked file on screen`() {
// The other half. Without it, a wiring with BOTH actions bound to reset passes the test
// above -- and that is exactly what a copy-paste of the wrong line produces.
installTestWorkManager(app, Data.EMPTY)
val pick = ParkedPickDispatcher()
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
viewModel.onInputPicked(INPUT_URI)
pick.runAll()
val before = viewModel.state.value
actions.onCancel()
assertEquals(
"Cancel must not throw away the pick the way Start over does",
before,
viewModel.state.value,
)
}
@Test
fun `each settings affordance reaches the setting it is named after`() {
// The typed bindings. The compiler already rejects a transposition among these, so this is
// not that assertion -- it is the cheaper one that each is bound to *something*, and that a
// binding dropped to `{}` during an edit would be caught.
installTestWorkManager(app, Data.EMPTY)
val pick = ParkedPickDispatcher()
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
actions.onPreset(OutputFormat.WEBM_VP9)
assertEquals(OutputFormat.WEBM_VP9.spec, viewModel.settings.value.spec)
actions.onContainer(Container.MKV)
assertEquals(Container.MKV, viewModel.settings.value.spec.container)
actions.onVideoCodec(VideoCodec.H264)
assertEquals(VideoCodec.H264, viewModel.settings.value.spec.videoCodec)
actions.onAudioCodec(AudioCodec.FLAC)
assertEquals(AudioCodec.FLAC, viewModel.settings.value.spec.audioCodec)
actions.onQuality(QualityTier.BEST)
assertEquals(QualityTier.BEST, viewModel.settings.value.quality)
actions.onEnginePreference(EnginePreference.FORCE_SOFTWARE)
assertEquals(EnginePreference.FORCE_SOFTWARE, viewModel.settings.value.enginePreference)
actions.onSuggestion(OutputFormat.MP4_H264.spec)
assertEquals(OutputFormat.MP4_H264.spec, viewModel.settings.value.spec)
}
@Test
fun `the launcher-backed actions are the ones the screen supplies`() {
// Not wired to the ViewModel at all, deliberately -- they need an ActivityResultLauncher.
// Asserted so that a later edit routing one of them at the ViewModel is noticed.
installTestWorkManager(app, Data.EMPTY)
val pick = ParkedPickDispatcher()
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
val called = mutableListOf<String>()
val actions = converterActions(
viewModel,
onPickInput = { called += "pick" },
onConvert = { called += "convert" },
onSave = { called += "save:$it" },
)
actions.onPickInput()
actions.onConvert()
actions.onSave("holiday.mp4")
assertEquals(listOf("pick", "convert", "save:holiday.mp4"), called)
}
// --- the join screen, where three are interchangeable -------------------
@Test
fun `Start over resets the join, and Cancel does not`() {
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
val pick = ParkedPickDispatcher()
val viewModel = JoinViewModel(app, pickDispatcher = pick)
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
viewModel.onInputsPicked(TWO_INPUTS)
pick.runAll()
assertTrue(
"the fixture needs a non-Idle state: ${viewModel.state.value}",
viewModel.state.value !is JoinState.Idle,
)
actions.onReset()
assertEquals(JoinState.Idle, viewModel.state.value)
}
@Test
fun `Cancel leaves the picked files on screen`() {
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
val pick = ParkedPickDispatcher()
val viewModel = JoinViewModel(app, pickDispatcher = pick)
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
viewModel.onInputsPicked(TWO_INPUTS)
pick.runAll()
val before = viewModel.state.value
actions.onCancel()
assertEquals(before, viewModel.state.value)
}
@Test
fun `Join starts the job rather than cancelling or resetting it`() {
// The third of the join screen's interchangeable trio, and the one whose transposition is
// worst: a Join button bound to cancel does nothing at all, which reads as a dead button.
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
val pick = ParkedPickDispatcher()
val viewModel = JoinViewModel(app, pickDispatcher = pick)
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
viewModel.onInputsPicked(TWO_INPUTS)
pick.runAll()
actions.onJoin()
assertTrue(
"Join must leave Ready for a running state, not sit still and not go Idle: " +
"${viewModel.state.value}",
viewModel.state.value is JoinState.Joining || viewModel.state.value is JoinState.Joined,
)
}
private companion object {
val INPUT_URI: Uri = Uri.parse("content://test/holiday.mov")
val TWO_INPUTS = listOf(
Uri.parse("content://test/a.mp4"),
Uri.parse("content://test/b.mp4"),
)
}
}
@@ -0,0 +1,197 @@
package org.libremediaconverter.convert
import android.app.Application
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertNull
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.QualityTier
import org.libremediaconverter.model.VideoCodec
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
/**
* The seven one-line edits the settings sheet makes, and what each one leaves alone.
*
* ## Why these needed a file of their own
*
* `setPreset` was covered. The six beside it — `setContainer`, `setVideoCodec`, `setAudioCodec`,
* `applySuggestion`, `setQuality`, `setEnginePreference` — and `cancel()` had **no coverage at
* all**, which is the tell: they are reachable from the JVM suite by exactly the route `setPreset`
* already takes, and nothing had asked.
*
* ## What is actually being asserted
*
* Not "the setter sets something". Each of these copies into a nested `OutputSpec`, so the failure
* worth catching is **a setter that writes the right value into the wrong field, or that rebuilds
* the spec and silently discards the other two**. So every test here asserts the field it changed
* *and* that the rest of the spec survived — a `setContainer` implemented as
* `it.copy(spec = OutputFormat.MP4_H265.spec.copy(container = container))` would pass a test that
* only checked the container.
*
* `ConverterScreenContentTest` cannot cover this: it builds `ConverterActions` itself and never
* touches the ViewModel. That the *screen* calls these is #156's, and neither implies the other.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class SettingsEditsTest {
private lateinit var app: Application
private lateinit var viewModel: ConversionViewModel
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
installTestWorkManager(app, Data.EMPTY)
viewModel = ConversionViewModel(app)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `choosing a preset replaces the whole spec`() {
viewModel.setPreset(OutputFormat.WEBM_VP9)
assertEquals(OutputFormat.WEBM_VP9.spec, viewModel.settings.value.spec)
}
@Test
fun `changing the container leaves both codecs alone`() {
// Moved off the default spec first, and that is load-bearing rather than tidiness. The
// default IS `OutputFormat.MP4_H265.spec`, so a `setContainer` that rebuilt the spec from
// that preset instead of from the current one produced an identical answer and the
// mutation went green. Editing the codecs away from the default first is what makes
// "the other two survived" an assertion rather than a coincidence.
viewModel.setPreset(OutputFormat.WEBM_VP9)
val before = viewModel.settings.value.spec
viewModel.setContainer(Container.MKV)
val after = viewModel.settings.value.spec
assertEquals(Container.MKV, after.container)
assertEquals("the video codec is not the container's to change", before.videoCodec, after.videoCodec)
assertEquals("the audio codec is not the container's to change", before.audioCodec, after.audioCodec)
}
@Test
fun `changing the video codec leaves the container and the audio codec alone`() {
// The transposition this guards against is real: setVideoCodec and setAudioCodec take
// different enum types, but a copy(...) naming the wrong field compiles wherever the types
// happen to line up, and the picker would silently set the other one.
val before = viewModel.settings.value.spec
viewModel.setVideoCodec(VideoCodec.VP9)
val after = viewModel.settings.value.spec
assertEquals(VideoCodec.VP9, after.videoCodec)
assertEquals(before.container, after.container)
assertEquals(before.audioCodec, after.audioCodec)
}
@Test
fun `changing the audio codec leaves the container and the video codec alone`() {
val before = viewModel.settings.value.spec
viewModel.setAudioCodec(AudioCodec.OPUS)
val after = viewModel.settings.value.spec
assertEquals(AudioCodec.OPUS, after.audioCodec)
assertEquals(before.container, after.container)
assertEquals(before.videoCodec, after.videoCodec)
}
@Test
fun `applying a suggestion replaces the spec without disturbing quality or engine`() {
// A suggestion comes from ContainerCapabilities when the current spec is invalid, so it is
// a whole spec by construction. What it must not do is reset the two settings beside it.
viewModel.setQuality(QualityTier.BEST)
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
viewModel.applySuggestion(OutputFormat.MKV_H264.spec)
val settings = viewModel.settings.value
assertEquals(OutputFormat.MKV_H264.spec, settings.spec)
assertEquals(QualityTier.BEST, settings.quality)
assertEquals(EnginePreference.FORCE_SOFTWARE, settings.enginePreference)
}
@Test
fun `changing the quality leaves the spec and the engine preference alone`() {
// Both neighbours are moved off their defaults first. Asserting against AUTO -- which is
// what `ConversionSettings` starts with -- let a `setQuality` that also reset the engine
// preference to AUTO pass, because the reset and the survival looked identical.
viewModel.setPreset(OutputFormat.WEBM_VP9)
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
val before = viewModel.settings.value.spec
viewModel.setQuality(QualityTier.BEST)
val settings = viewModel.settings.value
assertEquals(QualityTier.BEST, settings.quality)
assertEquals(before, settings.spec)
assertEquals(
"quality is not the engine preference's to change",
EnginePreference.FORCE_SOFTWARE,
settings.enginePreference,
)
}
@Test
fun `changing the engine preference leaves the spec and the quality alone`() {
// Off the defaults for the same reason as the test above: QualityTier.FAST is the starting
// value, so asserting it here would have been satisfied by a reset as readily as by a
// survival.
viewModel.setPreset(OutputFormat.WEBM_VP9)
viewModel.setQuality(QualityTier.BEST)
val before = viewModel.settings.value.spec
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
val settings = viewModel.settings.value
assertEquals(EnginePreference.FORCE_SOFTWARE, settings.enginePreference)
assertEquals(before, settings.spec)
assertEquals(
"the engine preference is not the quality's to change",
QualityTier.BEST,
settings.quality,
)
}
@Test
fun `editing past every preset leaves no matching preset`() {
// `matchingPreset` is what the settings sheet reads to decide whether to show a preset as
// selected or to say "Custom". Editing one field of a preset must drop it out of the list
// rather than leaving the old one highlighted.
viewModel.setPreset(OutputFormat.MP4_H265)
assertEquals(OutputFormat.MP4_H265, viewModel.settings.value.matchingPreset)
viewModel.setAudioCodec(AudioCodec.FLAC)
assertNull(
"an edited spec is no longer any preset, and the sheet says Custom",
viewModel.settings.value.matchingPreset,
)
assertNotEquals(OutputFormat.MP4_H265.spec, viewModel.settings.value.spec)
}
@Test
fun `cancelling with no active job does nothing rather than throwing`() {
// `activeWorkId?.let(...)` -- the null side. A user can reach Cancel through a state that
// has already finished, and taking the app down for it would be worse than doing nothing.
viewModel.cancel()
assertEquals(ConversionState.Idle, viewModel.state.value)
}
}
@@ -0,0 +1,70 @@
package org.libremediaconverter.convert
import android.net.Uri
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.ConcatPlanner
import org.libremediaconverter.model.ConcatStrategy
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
/**
* A clip in a join that nothing could read, from the probe all the way to the strategy.
*
* Both halves of this are covered already, and separately: `MediaProbeTrackWalkTest` pins what
* `concatInputFrom` makes of a track list, and `ConcatPlannerTest`'s
* `an unknown codec is not treated as a match` pins what the planner does with a hand-built
* `ConcatInput(video = null)`. **Nothing spanned the two**, and the span is the load-bearing part:
* the planner's safety rests on the probe really producing that shape, and the hand-built fixture
* would go on passing if it stopped.
*
* Measured rather than asserted: mutating `concatInputFrom`'s initial `video` to a non-null
* placeholder leaves `ConcatPlannerTest` green and turns this red.
*
* ## The asymmetry this protects
*
* `ConcatPlanner` guards its video check against a null codec (`ConcatStrategy.kt:51`) and its
* audio check not at all (`:54`). **That is correct, not an oversight.** `MediaProbe.shortName`
* returns a non-null `String`, so in `concatInputFrom` a null `audioCodec` means the track is
* *absent* — and two clips with no audio genuinely do match. A null `videoCodec` carries both
* meanings, absent or unreadable, which is why only that one is guarded.
*
* So the audio check is safe *because* the video guard fires first on a clip nothing could read.
* Nothing wrote that coupling down and nothing held it.
*
* ## What this deliberately does not cover
*
* `probeForConcat`'s `catch` arm (`MediaProbe.kt:300-302`). It is **not reachable on the JVM**:
* Robolectric's `MediaExtractor` never throws from `setDataSource`, measured across an
* unregistered `content://` authority, a missing `file://`, a file of garbage bytes and an `http://`
* URL — all four returned normally with `trackCount = 0`. So the failure arrives here as an empty
* track list rather than as an exception, which reaches the same `ConcatInput(null, null, 0, 0, 0)`
* by the other road. The catch stays device-only, and this file does not pretend otherwise.
*/
@RunWith(RobolectricTestRunner::class)
class UnreadableJoinInputTest {
@Test
fun `a clip nothing could read probes as unknown, and an unknown clip is re-encoded`() {
val unreadable = MediaProbe.probeForConcat(RuntimeEnvironment.getApplication(), UNREADABLE)
assertNull("an unreadable clip proves nothing about its video codec", unreadable.videoCodec)
assertNull("nor about its audio codec", unreadable.audioCodec)
assertEquals("nor about its dimensions", 0, unreadable.width)
assertEquals(0, unreadable.height)
assertEquals(0, unreadable.frameRate)
assertEquals(
"a clip nothing could read is not evidence of a match with anything",
ConcatStrategy.REENCODE,
ConcatPlanner.plan(listOf(unreadable, unreadable)),
)
}
private companion object {
/** `content://` so the probe takes the SAF branch a real pick takes. Nothing answers it. */
val UNREADABLE: Uri = Uri.parse("content://test/vanished.mp4")
}
}
@@ -166,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)
@@ -0,0 +1,200 @@
package org.libremediaconverter.join
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.WorkInfo
import androidx.work.workDataOf
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.InputFile
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.work.ConcatWorker
import org.robolectric.RobolectricTestRunner
/**
* Every answer [joinStateFrom] can give, chosen rather than stumbled into.
*
* The join-side twin of `ConversionStateMappingTest`, and the argument is the same one: the mapping
* ran on every test that drove a real `ConcatWorker`, but a real worker only ever reaches a terminal
* state with well-formed output, so five arms had never been *chosen* by anything.
*
* ## The one that is not just coverage
*
* `an unknown strategy name is read as a re-encode rather than thrown` covers a real defect this
* seam exposed. The line it replaces was:
*
* ```kotlin
* .getString(ConcatWorker.KEY_STRATEGY)?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE
* ```
*
* `valueOf` throws on a name this build does not define, and this runs inside a `viewModelScope`
* collect with no handler — so it does not become a `Failed` state, it takes the process down.
* `ConcatWorker.kt` had already made this exact change for `KEY_FORMAT` and written down why; the
* matching read on this side had not been changed with it.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class JoinStateMappingTest {
@Test
fun `a running join is joining`() {
assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.RUNNING))
}
@Test
fun `a blocked join looks like one that is starting`() {
// Folded into the RUNNING arm deliberately: a job waiting on a prerequisite is nothing the
// user can act on, and a separate word for it would be noise.
assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.BLOCKED))
}
@Test
fun `an enqueued join that has already run is waiting to retry`() {
assertEquals(JoinState.Waiting(INPUTS), map(WorkInfo.State.ENQUEUED, runAttemptCount = 1))
}
@Test
fun `an enqueued join that has never run is simply starting`() {
// The other side. Without it, a mapping that ignored runAttemptCount passes the test above.
assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.ENQUEUED, runAttemptCount = 0))
}
@Test
fun `a success that named no file is a failure, not an empty success`() {
assertEquals(
JoinState.Failed(JOINED_WITHOUT_A_FILE_MESSAGE),
map(WorkInfo.State.SUCCEEDED, data = Data.EMPTY),
)
}
@Test
fun `a success carries the strategy the worker actually used`() {
// Not cosmetic: the join screen tells the user whether their files were stream-copied or
// re-encoded, which is the difference between lossless and lossy.
val joined = map(
WorkInfo.State.SUCCEEDED,
data = workDataOf(
ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
ConcatWorker.KEY_STRATEGY to ConcatStrategy.STREAM_COPY.name,
),
) as JoinState.Joined
assertEquals(ConcatStrategy.STREAM_COPY, joined.strategy)
}
@Test
fun `an unknown strategy name is read as a re-encode rather than thrown`() {
// The defect. A build that added a third strategy leaves finished joins in the queue naming
// it, and WorkManager keeps those about a week -- the premise WorkerEnumFallbackTest and
// JobTags are both written on. With `valueOf` this throws IllegalArgumentException inside a
// viewModelScope collect that has no handler, so it is not a Failed state, it is a crash.
//
// REENCODE rather than STREAM_COPY because it is the conservative answer: describing an
// unknown join as lossless would be a claim the app cannot support.
val joined = map(
WorkInfo.State.SUCCEEDED,
data = workDataOf(
ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
ConcatWorker.KEY_STRATEGY to "SMART_CONCAT_V2",
),
) as JoinState.Joined
assertEquals(ConcatStrategy.REENCODE, joined.strategy)
}
@Test
fun `a success with no strategy at all falls back the same way`() {
val joined = map(
WorkInfo.State.SUCCEEDED,
data = workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4"),
) as JoinState.Joined
assertEquals(ConcatStrategy.REENCODE, joined.strategy)
}
@Test
fun `a success from older work falls back to the format such a job really used`() {
val joined = map(
WorkInfo.State.SUCCEEDED,
data = workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4"),
) as JoinState.Joined
assertEquals(ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT), joined.suggestedName)
assertEquals(ConcatWorker.DEFAULT_FORMAT.mimeType, joined.mimeType)
}
@Test
fun `a blank name or type falls back the same way a missing one does`() {
val joined = map(
WorkInfo.State.SUCCEEDED,
data = workDataOf(
ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
ConcatWorker.KEY_SUGGESTED_NAME to "",
ConcatWorker.KEY_MIME_TYPE to " ",
),
) as JoinState.Joined
assertEquals(ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT), joined.suggestedName)
assertEquals(ConcatWorker.DEFAULT_FORMAT.mimeType, joined.mimeType)
}
@Test
fun `a failure carries the reason the worker gave`() {
assertEquals(
JoinState.Failed("Not enough free space to join these files."),
map(
WorkInfo.State.FAILED,
data = workDataOf(
ConcatWorker.KEY_ERROR to "Not enough free space to join these files.",
),
),
)
}
@Test
fun `a failure with nothing said still says something`() {
assertEquals(
JoinState.Failed(ConcatWorker.GENERIC_FAILURE_MESSAGE),
map(WorkInfo.State.FAILED, data = Data.EMPTY),
)
}
@Test
fun `a failure whose message is blank falls back like a missing one`() {
assertEquals(
JoinState.Failed(ConcatWorker.GENERIC_FAILURE_MESSAGE),
map(WorkInfo.State.FAILED, data = workDataOf(ConcatWorker.KEY_ERROR to " ")),
)
}
@Test
fun `a cancellation lands wherever the caller said it should`() {
// A join started here goes back to Ready with the picked files; one picked up by reattach
// goes to Idle, because those URIs belong to a process that no longer exists.
assertEquals(
JoinState.Ready(INPUTS),
map(WorkInfo.State.CANCELLED, cancelled = JoinState.Ready(INPUTS)),
)
assertEquals(JoinState.Idle, map(WorkInfo.State.CANCELLED, cancelled = JoinState.Idle))
}
private fun map(
state: WorkInfo.State,
runAttemptCount: Int = 0,
data: Data = Data.EMPTY,
cancelled: JoinState = JoinState.Ready(INPUTS),
): JoinState = joinStateFrom(
JoinUpdate(state = state, runAttemptCount = runAttemptCount, outputData = data),
inputs = INPUTS,
cancelled = cancelled,
)
private companion object {
val INPUTS = listOf(
InputFile(Uri.parse("content://test/a.mp4"), "a.mp4", 1024L),
InputFile(Uri.parse("content://test/b.mp4"), "b.mp4", 2048L),
)
}
}
@@ -0,0 +1,102 @@
package org.libremediaconverter.join
import android.app.Application
import android.net.Uri
import androidx.media3.common.util.UnstableApi
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.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.RecordingPublisher
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.work.ConcatWorker
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
/**
* The two layers that refuse a short join, refusing it with one sentence.
*
* ## Why this is not "assert a constant equals itself"
*
* `ConcatWorker` and `JoinViewModel` both reject a join of fewer than two files, and before #158
* each carried **its own copy of the literal**. Only the worker's was pinned — by `RefusedJobTest`,
* added in #139 — so the wording on the screen could drift away from the wording in the job with no
* test saying anything, for one message the user sees from one condition.
*
* Sharing a constant makes them agree by construction. What it does *not* do is prove that both
* layers still reach it: a refactor that stops `JoinViewModel` refusing at all, or that gives it a
* different message, passes any test that only reads `TOO_FEW_INPUTS_MESSAGE`. So each layer is
* driven for real here — the ViewModel through `onInputsPicked`, the worker through `doWork` — and
* the assertion is that the two answers are **the same string**, taken from two running layers
* rather than from one declaration.
*
* That is the shape `CLAUDE.md` asks for: revert the sharing and this goes red, because the two
* sites drift the moment they are allowed to.
*
* ## Scope
*
* The arity guard's own behaviour on the ViewModel side — that it refuses one file, that it accepts
* two, that it claims ownership first — is #155's, and this deliberately does not duplicate it.
* This file is about the *agreement between layers*, which is what #158 changed.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class SharedFailureMessagesTest {
private lateinit var app: Application
private lateinit var viewModel: JoinViewModel
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
ConversionDependencies.publisher = { RecordingPublisher(app) }
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
viewModel = JoinViewModel(app)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `both layers refuse a one-file join with the same sentence`() {
// The ViewModel, refusing before anything is enqueued.
viewModel.onInputsPicked(listOf(ONE_FILE))
val fromScreen = (viewModel.state.value as JoinState.Failed).message
// The worker, refusing a job that reached the queue anyway -- which it can, because
// ConcatWorker.request(...) takes a List<Uri> and checks nothing about its length.
val result = runBlocking { worker(ONE_FILE).doWork() }
val fromJob = (result as ListenableWorker.Result.Failure)
.outputData.getString(ConcatWorker.KEY_ERROR)
assertEquals(
"the screen and the job must say the same thing about the same refusal",
fromScreen,
fromJob,
)
// And that the shared sentence is the one either layer would have written on its own,
// rather than both having drifted together to something else.
assertEquals(ConcatWorker.TOO_FEW_INPUTS_MESSAGE, fromScreen)
}
private fun worker(vararg inputs: Uri): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(
ConcatWorker.KEY_INPUT_URIS to inputs.map(Uri::toString).toTypedArray(),
ConcatWorker.KEY_TOTAL_BYTES to 1024L,
),
runAttemptCount = 0,
).build()
private companion object {
val ONE_FILE: Uri = Uri.parse("content://test/holiday.mp4")
}
}
@@ -358,4 +358,213 @@ class ContainerCapabilitiesTest {
assertEquals(emptyList<VideoCodec>(), ContainerCapabilities.encodableVideo(container))
}
}
// --- the audio axis -----------------------------------------------------
//
// Every rule below has a video twin already tested above. The two halves of `validate` were
// written together and only one of them was ever checked, so these are deliberately shaped like
// their twins rather than as a fresh idea about what to assert.
@Test
fun `an unidentifiable source audio codec cannot be copied`() {
// The audio twin of `an unidentifiable source codec cannot be copied`. Never guess: a copy
// of an unidentified codec is how you ship a file that does not play.
val unknownAudio = InputProbe(videoCodec = "h264", audioCodec = null, container = Container.MP4)
val spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.COPY)
val invalid = ContainerCapabilities.validate(spec, unknownAudio) as? Validation.Invalid
?: throw AssertionError("copying an unidentified audio codec must be refused")
assertTrue(invalid.message, invalid.message.contains("could not be identified"))
assertEverySuggestionValid(invalid, unknownAudio)
}
@Test
fun `copying an audio codec the container cannot hold is refused`() {
// MP4 carries AAC, MP3, Opus and FLAC. Vorbis lives in Ogg and Matroska, so a stream copy
// out of a Vorbis source into MP4 has nowhere to put the track.
val vorbisAudio = InputProbe(videoCodec = "h264", audioCodec = "vorbis", container = Container.MKV)
val spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.COPY)
val invalid = ContainerCapabilities.validate(spec, vorbisAudio) as? Validation.Invalid
?: throw AssertionError("Vorbis copied into MP4 must be refused")
assertEquals("MP4 cannot hold Vorbis audio.", invalid.message)
assertEverySuggestionValid(invalid, vorbisAudio)
}
@Test
fun `an audio codec the container cannot hold is refused on the encode path too`() {
// WAV carries PCM and nothing else. The twin is `H265 in AVI is refused`.
val spec = OutputSpec(Container.WAV, VideoCodec.NONE, AudioCodec.AAC)
val invalid = ContainerCapabilities.validate(spec, mp3Source) as? Validation.Invalid
?: throw AssertionError("AAC in WAV must be refused")
assertEquals("WAV cannot hold AAC audio.", invalid.message)
assertEverySuggestionValid(invalid, mp3Source)
}
@Test
fun `an audio codec this app cannot encode is refused, and copying is offered instead`() {
// Matroska carries Vorbis; nothing here encodes it. The refusal has to say so *and* say
// what would work, which is the audio twin of `copying is offered as the fix when the codec
// is right but unencodable`.
val spec = OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.VORBIS)
val invalid = ContainerCapabilities.validate(spec, h264Source) as? Validation.Invalid
?: throw AssertionError("encoding Vorbis must be refused")
assertEquals(
"This app cannot encode Vorbis audio. It can still be copied from a Vorbis source.",
invalid.message,
)
assertEverySuggestionValid(invalid, h264Source)
}
@Test
fun `copying a video codec the container cannot hold is refused`() {
// Not the audio axis, but the one video refusal with no test: AVI predates H.265, so a
// stream copy out of an HEVC source into AVI has nowhere to put the track. `H265 in AVI is
// refused` covers the matrix; this covers what validate() does with it.
val h265Source = InputProbe(videoCodec = "hevc", audioCodec = "mp3", container = Container.MP4)
val spec = OutputSpec(Container.AVI, VideoCodec.COPY, AudioCodec.MP3)
val invalid = ContainerCapabilities.validate(spec, h265Source) as? Validation.Invalid
?: throw AssertionError("H.265 copied into AVI must be refused")
assertEquals("AVI cannot hold H.265 video.", invalid.message)
assertEverySuggestionValid(invalid, h265Source)
}
@Test
fun `no audio track is accepted by every container in both modes`() {
// The audio twin of VideoCodec.NONE -> true. A container that refused "no audio" would make
// every video-only output invalid.
Container.entries.forEach { container ->
listOf(CodecMode.COPY, CodecMode.ENCODE).forEach { mode ->
assertTrue(
"$container should accept no audio track ($mode)",
ContainerCapabilities.accepts(container, AudioCodec.NONE, mode),
)
}
}
}
/**
* The video twin of `no audio track is accepted by every container in both modes`.
*
* Dead in production today, and deliberately so: every caller guards `NONE` before asking the
* matrix, so nothing reaches this arm through the app. **The asymmetry is the argument, not the
* reachability** -- its audio counterpart at the top of the same `when` has had a dedicated
* test since #136, and one of a matched pair being covered is how a later reader concludes the
* other was considered and exempted. It was not; it was simply missed.
*
* Not the same shape as the two `COPY -> error(...)` arms, which `docs/coverage-read-findings.md`
* records as a named exemption (F4). Those are guards that must not be provokable. This is a
* documented answer -- "no video track fits anywhere" -- and an answer is a thing to pin.
*/
@Test
fun `no video track is accepted by every container in both modes`() {
Container.entries.forEach { container ->
listOf(CodecMode.COPY, CodecMode.ENCODE).forEach { mode ->
assertTrue(
"$container should accept no video track ($mode)",
ContainerCapabilities.accepts(container, VideoCodec.NONE, mode),
)
}
}
}
/**
* A suggestion that keeps the codec the user asked for, rather than falling back to the
* container's first encodable one.
*
* `repairVideo`'s third arm -- "the request is not a copy, and this container can encode it" --
* is the one that preserves intent, and it was the only arm of the four nothing reached. The
* property test above executes `repairVideo` on every case it walks and lands elsewhere each
* time: an explicit COPY that works, a source the container can carry untouched, or no video
* track at all.
*
* The route is indirect because it is the only one the app has. VP9 into WebM is a perfectly
* good video request; what makes it invalid is the *audio* -- WebM carries Opus and Vorbis, not
* AAC. So `validateAudio` refuses, `suggestions` looks for a container that can hold what was
* asked for, and MP4 can encode VP9. The suggestion has to come back carrying VP9: swapping to
* the container's first encodable codec would discard the choice the user made.
*/
@Test
fun `a repaired suggestion keeps the video codec the user chose`() {
val invalid = ContainerCapabilities.validate(
OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.AAC),
h264Source,
)
assertTrue("WebM cannot hold AAC, so this spec is invalid", invalid is Validation.Invalid)
val suggestions = (invalid as Validation.Invalid).suggestions
assertTrue(
"expected a suggestion that still encodes VP9, got $suggestions",
suggestions.any { it.videoCodec == VideoCodec.VP9 },
)
assertEverySuggestionValid(invalid, h264Source)
}
/**
* The fallback in `firstContainerHolding`: when the input's own container cannot hold the
* codec the user asked for, any container that can will do.
*
* The preferred half -- "the container the input already uses" -- is what every other case
* reaches, because they all start from a file whose own container carries the codec in
* question. The elvis after it had never run.
*
* AVI is the input that makes it run: AVI predates H.265 and has no mapping for it, so asking
* an AVI for H.265 is refused, and the container the input already uses cannot be part of the
* answer. Without the fallback the only candidates left are AVI itself and the container
* holding the *source* codec -- also AVI -- so the refusal still offers something, but what it
* offers is H.264: the app quietly declines the codec the user asked for instead of moving them
* to a container that supports it.
*
* That is why this asserts the codec survives rather than that the list is non-empty. A
* non-empty assertion passes with the fallback deleted -- measured, not assumed.
*/
@Test
fun `an input whose container cannot hold the requested codec is moved, not downgraded`() {
val aviSource = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.AVI)
val invalid = ContainerCapabilities.validate(
OutputSpec(Container.AVI, VideoCodec.H265, AudioCodec.AAC),
aviSource,
)
assertTrue("AVI has no mapping for H.265", invalid is Validation.Invalid)
val suggestions = (invalid as Validation.Invalid).suggestions
assertTrue(
"expected a container that can actually hold H.265, got $suggestions",
suggestions.any { it.videoCodec == VideoCodec.H265 },
)
assertEverySuggestionValid(invalid, aviSource)
}
@Test
fun `resolving audio COPY before asking the matrix is required`() {
// The audio twin of `resolving COPY before asking the matrix is required`, and the reason is
// identical: silently answering "false" would refuse a perfectly good remux.
runCatching { ContainerCapabilities.accepts(Container.MP4, AudioCodec.COPY, CodecMode.COPY) }
.onSuccess { throw AssertionError("expected audio COPY to be rejected by the matrix") }
}
/**
* Every alternative a refusal offers has to be one the same input could actually take.
*
* `Validation.Invalid` promises exactly this and names this class as the proof. The global
* property test walks the presets; these paths reach `suggestions()` through `validateAudio`,
* which no preset does.
*/
private fun assertEverySuggestionValid(invalid: Validation.Invalid, probe: InputProbe) {
invalid.suggestions.forEach {
assertTrue(
"suggestion $it is itself invalid, so the chip leads to a second error",
ContainerCapabilities.validate(it, probe).isValid,
)
}
}
}
@@ -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,
)
@@ -27,9 +27,6 @@ import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.UUID
import java.util.concurrent.ExecutionException
import java.util.concurrent.Executor
import java.util.concurrent.TimeUnit
/**
* That a refused foreground-service start does not end the job.
@@ -125,6 +122,40 @@ class DeniedForegroundStartTest {
)
}
@Test
fun `a join denied past the attempt bound fails with a message the user can act on`() {
// The join twin of the conversion case above. ConcatWorker reaches the same FailureOutcome
// through its own `when`, and that arm was the only one of its three with no test -- so a
// join that gave up silently, or gave up with an empty Data, would have looked identical to
// one that retried.
val worker = concatWorker(runAttemptCount = FailureOutcome.MAX_FOREGROUND_START_ATTEMPTS)
val result = runBlocking { worker.doWork() }
assertEquals(
ListenableWorker.Result.failure(
workDataOf(ConcatWorker.KEY_ERROR to FailureOutcome.FOREGROUND_DENIED_MESSAGE),
),
result,
)
}
@Test
fun `a join that gives up collects the partial it had already staged`() {
// The delete lives on ConcatWorker's `catch (e: Throwable)` path, which every give-up goes
// through. Written first so a missing delete cannot pass by asking whether a file nobody
// wrote is absent.
concatStagedFile().writeBytes(ByteArray(PARTIAL_BYTES))
runBlocking { concatWorker(runAttemptCount = FailureOutcome.MAX_FOREGROUND_START_ATTEMPTS).doWork() }
assertEquals(
"a join that gave up must not orphan what it staged",
emptyList<String>(),
stagedNames(),
)
}
private fun conversionWorker(runAttemptCount: Int = 0): ConversionWorker =
TestListenableWorkerBuilder<ConversionWorker>(
context = app,
@@ -141,18 +172,22 @@ class DeniedForegroundStartTest {
.setForegroundUpdater(DenyingForegroundUpdater)
.build()
private fun concatWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
private fun concatWorker(runAttemptCount: Int = 0): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(
ConcatWorker.KEY_INPUT_URIS to arrayOf(INPUT.toString(), "content://test/second.mp4"),
ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES,
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
ConcatWorker.KEY_FORMAT to CONCAT_FORMAT.name,
),
runAttemptCount = 0,
runAttemptCount = runAttemptCount,
).setId(CONCAT_ID)
.setForegroundUpdater(DenyingForegroundUpdater)
.build()
/** The staging path the join will compute, asked for rather than spelled out here. */
private fun concatStagedFile(): File =
publisher.createStagingFile(StagingNames.forJob(CONCAT_ID, CONCAT_FORMAT.extension))
/** The staging path the worker will compute, asked for rather than spelled out here. */
private fun stagedFile(): File = publisher.createStagingFile(StagingNames.forJob(CONVERSION_ID, SPEC.extension))
@@ -164,6 +199,7 @@ class DeniedForegroundStartTest {
const val INPUT_BYTES = 1024L
const val PARTIAL_BYTES = 2048
val SPEC = OutputFormat.MP4_H265.spec
val CONCAT_FORMAT = OutputFormat.MP4_H264
val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000001")
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000002")
}
@@ -182,18 +218,3 @@ private object DenyingForegroundUpdater : ForegroundUpdater {
),
)
}
/**
* An already-failed future, written out rather than pulled from a futures library.
*
* `await()` takes the `isDone` fast path and unwraps the `ExecutionException`, which is what puts
* the platform's own exception in front of the worker's catch rather than a wrapper.
*/
private class FailedFuture(private val failure: Throwable) : ListenableFuture<Void> {
override fun addListener(listener: Runnable, executor: Executor): Unit = executor.execute(listener)
override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false
override fun isCancelled(): Boolean = false
override fun isDone(): Boolean = true
override fun get(): Void = throw ExecutionException(failure)
override fun get(timeout: Long, unit: TimeUnit): Void = throw ExecutionException(failure)
}
@@ -0,0 +1,88 @@
package org.libremediaconverter.work
import android.content.pm.ServiceInfo
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
/**
* [ConversionForegroundType.current] answers differently on each of the three API regimes, and
* until this file only one of them was ever executed.
*
* `app/src/test/resources/robolectric.properties` pins the whole JVM suite to `sdk=36`, so every
* Robolectric test that reaches a `ForegroundInfo` takes the `mediaProcessing` arm and no other.
* The 33 and 34 arms were cold: 3 lines and 3 of 4 branches, measured on `main` at `d354f64`.
*
* **The instrumented test is not a substitute, and the reason is specific.**
* `ConversionWorkerTest.foregroundTypeMatchesTheRunningApiLevel` asserts against whichever API the
* leg happens to be — one arm per leg, never the other two — and the legs that would cover 33 and
* 34 are the ones issue #122 wedges. `docs/coverage-read-findings.md` records an API 33 run that
* reported `received: 60` and `failed: unknown`: the regime *was* exercised, and that leg could
* not have said so if it had broken. Four `@Config` classes here pin all three arms
* deterministically, in the same `./gradlew` invocation as everything else.
*
* `minSdk` is 33, so none of these is dead code — each is a device someone is running the app on.
*
* **SDK 35 is in the list for the boundary, not for the answer.** It shares its answer with 36,
* which would make it look redundant. It is not: relaxing `>= VANILLA_ICE_CREAM` to `>` is invisible
* at every level except exactly 35, so without this class that mutation survives the suite.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [33])
class ForegroundTypeApi33Test {
/**
* Zero rather than a named constant because there is no constant to name: API 33 does not
* require a type, and `mediaProcessing` does not exist here to pass. `ForegroundInfo` reads 0
* as "no type at all", which is what this regime wants.
*/
@Test
fun `api 33 asks for no foreground service type`() {
assertEquals(0, ConversionForegroundType.current())
}
}
/**
* API 34 makes a type mandatory and still has no `mediaProcessing`, so `dataSync` is the only
* sensible fit. See [ForegroundTypeApi33Test] for why this file exists.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34])
class ForegroundTypeApi34Test {
@Test
fun `api 34 falls back to dataSync, the only type that fits`() {
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC, ConversionForegroundType.current())
}
}
/**
* The first level with `mediaProcessing`, and therefore the one that tells `>=` from `>`.
* See [ForegroundTypeApi33Test].
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [35])
class ForegroundTypeApi35Test {
@Test
fun `api 35 is the first level that takes mediaProcessing`() {
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING, ConversionForegroundType.current())
}
}
/**
* The level the rest of the suite runs at, asserted here rather than assumed — it is the one arm
* that was already covered, and leaving it out would make this file look like it is about the old
* levels rather than about all three regimes. See [ForegroundTypeApi33Test].
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [36])
class ForegroundTypeApi36Test {
@Test
fun `api 36 keeps mediaProcessing`() {
assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING, ConversionForegroundType.current())
}
}
@@ -0,0 +1,226 @@
package org.libremediaconverter.work
import android.app.Application
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.ListenableWorker
import androidx.work.testing.TestListenableWorkerBuilder
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertThrows
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.HardwareTranscoder
import org.libremediaconverter.convert.SoftwareTranscoder
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.DeviceCodecs
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.UUID
/**
* What happens when the hardware engine does not finish the job.
*
* `runMedia3OrFallBack` was eleven lines at 0% on the JVM and `isCancellation` had never been
* called by any unit test at all. Its own KDoc calls the fallback the protection against vendor
* hardware encoders that "cannot be tested for correctness", so it is the branch most likely to
* matter on a device nobody here owns — and it was reachable the whole time through
* `ConversionDependencies.hardware`, which no unit test had ever used.
*
* The sharp one is cancellation. `runMedia3OrFallBack` catches `Throwable`, so without the
* `isCancellation` re-throw a user cancelling a hardware transcode would have the app quietly
* start a *second* conversion in software — the one thing cancelling is supposed to prevent.
*
* `ForcedFailureTest` covers the failure half on a device. It does not cover the cancellation half,
* and this host cannot run it either way.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class HardwareFallbackTest {
private lateinit var app: Application
private lateinit var hardware: RecordingHardwareTranscoder
private lateinit var software: RecordingSoftwareTranscoder
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
hardware = RecordingHardwareTranscoder()
software = RecordingSoftwareTranscoder()
ConversionDependencies.publisher = { AlwaysRoomPublisher(app) }
ConversionDependencies.hardware = { hardware }
ConversionDependencies.software = { software }
// A probe with real codecs, not the default: `InputProbe()` reports UNPARSEABLE, which
// PERMISSIVE.canDecode refuses, and the router would send every job here straight to
// FFmpeg without any of these tests mentioning why.
ConversionDependencies.probe = { _, _ -> H264_SOURCE }
ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }
installTestWorkManager(app, Data.EMPTY)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `a hardware failure runs the job again in software, on a clean staging file`() {
hardware.failWith = { error("the vendor encoder produced nothing usable") }
val result = runBlocking { worker().doWork() }
assertTrue("the job should still succeed, got $result", result is ListenableWorker.Result.Success)
assertEquals("the hardware engine gets exactly one attempt", 1, hardware.attempts)
assertEquals("and the job then goes to software", 1, software.attempts)
// The `staged.delete()` between the two, asserted where it is observable: FFmpeg must not
// find a half-written hardware output sitting at the path it is about to write.
assertFalse(
"the partial hardware output must be gone before FFmpeg starts",
software.outputExistedOnEntry,
)
assertEquals("the hardware engine is closed either way", 1, hardware.closes)
}
@Test
fun `a cancelled hardware transcode is not quietly retried in software`() {
hardware.failWith = { throw CancellationException("the user pressed Cancel") }
assertThrows(CancellationException::class.java) { runBlocking { worker().doWork() } }
assertEquals("the hardware engine ran", 1, hardware.attempts)
assertEquals(
"cancelling must not start a second conversion -- that is the whole point of cancelling",
0,
software.attempts,
)
assertEquals("and the engine is still closed on the way out", 1, hardware.closes)
}
@Test
fun `a hardware transcode that works never reaches the software engine`() {
val result = runBlocking { worker().doWork() }
assertTrue("got $result", result is ListenableWorker.Result.Success)
assertEquals(1, hardware.attempts)
assertEquals("the fallback is a fallback, not a second pass", 0, software.attempts)
assertEquals(1, hardware.closes)
}
/**
* #169: the display-name fallback, which reaches further than the notification title.
*
* `inputData.getString(KEY_DISPLAY_NAME) ?: "input"` had never taken its right-hand side. The
* value is not only the foreground notification's title: it feeds `outputNameFor`, so it is
* also the filename offered in the user's save dialog. A job enqueued by an older build, or
* built by hand, carries no such key.
*/
@Test
fun `a job that names no input file still suggests an output name`() {
val result = runBlocking { worker(displayName = null).doWork() }
assertTrue("got $result", result is ListenableWorker.Result.Success)
val suggested = (result as ListenableWorker.Result.Success)
.outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME)
assertTrue(
"expected a name built from the fallback, got $suggested",
suggested.orEmpty().startsWith("input"),
)
}
private fun worker(displayName: String? = DISPLAY_NAME): ConversionWorker {
val spec = OutputFormat.MP4_H265.spec
val entries = buildMap<String, Any> {
put(ConversionWorker.KEY_INPUT_URI, INPUT.toString())
displayName?.let { put(ConversionWorker.KEY_DISPLAY_NAME, it) }
put(ConversionWorker.KEY_SIZE_BYTES, INPUT_BYTES)
put(ConversionWorker.KEY_CONTAINER, spec.container.name)
put(ConversionWorker.KEY_VIDEO_CODEC, spec.videoCodec.name)
put(ConversionWorker.KEY_AUDIO_CODEC, spec.audioCodec.name)
// AUTO rather than FORCE_SOFTWARE, which is what every other worker test uses and is
// exactly why this path had no coverage: forcing software never enters the function.
put(ConversionWorker.KEY_ENGINE_PREFERENCE, EnginePreference.AUTO.name)
}
return TestListenableWorkerBuilder<ConversionWorker>(
context = app,
inputData = Data.Builder().putAll(entries).build(),
runAttemptCount = 0,
).setId(JOB_ID).build()
}
private companion object {
val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4")
const val DISPLAY_NAME = "holiday.mp4"
const val INPUT_BYTES = 1024L
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000009")
val H264_SOURCE = InputProbe(
videoCodec = "h264",
audioCodec = "aac",
container = Container.MP4,
durationMs = 1_000,
)
}
}
/**
* A hardware engine that writes something before it fails, and remembers being closed.
*
* Writing first is the point, exactly as it is for `PartialThenFailingTranscoder`: an engine that
* only threw would let a missing `staged.delete()` pass unnoticed.
*/
@UnstableApi
private class RecordingHardwareTranscoder : HardwareTranscoder {
var attempts = 0
var closes = 0
var failWith: (() -> Unit)? = null
override suspend fun transcode(input: Uri, output: File, request: ConversionRequest, onProgress: (Int) -> Unit) {
attempts++
output.writeBytes(ByteArray(PARTIAL_BYTES))
failWith?.invoke()
}
override fun close() {
closes++
}
private companion object {
const val PARTIAL_BYTES = 2048
}
}
/** The software engine, recording whether the hardware attempt's leftovers were cleared first. */
private class RecordingSoftwareTranscoder : SoftwareTranscoder {
var attempts = 0
var outputExistedOnEntry = false
override suspend fun run(
request: ConversionRequest,
inputPath: String,
output: File,
durationMs: Long,
onProgress: (Int) -> Unit,
) {
attempts++
outputExistedOnEntry = output.exists()
output.writeBytes(ByteArray(OUTPUT_BYTES))
}
private companion object {
const val OUTPUT_BYTES = 512
}
}
@@ -16,6 +16,7 @@ import androidx.work.testing.WorkManagerTestInitHelper
import androidx.work.workDataOf
import kotlinx.coroutines.runBlocking
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
@@ -125,6 +126,39 @@ class JobSnapshotsTest {
assertEquals(newer.absolutePath, Reattachment.choose(snapshots)?.job?.outputPath)
}
/**
* A job in the tag query that never recorded an output path at all.
*
* Distinct from the three cases above, which all *have* a path and differ in what it names. A
* job still running, or one that finished without writing its result key, carries no path at
* all -- and `getWorkInfosByTagFlow` returns it alongside the finished ones, because the tag is
* the worker class and every attempt ever enqueued carries it.
*
* The guard is the `?.` in `path?.let(::File)`. Without it the null goes straight into a `File`
* constructor. What this pins is the consequence rather than the null check: such a job must
* not be offered as a result, so `Reattachment.choose` has to walk past it to the job that
* really produced a file. Choosing it would put a Converted screen in front of the user with a
* Save button that has nothing to save.
*/
@Test
fun `a job that recorded no output path is not offered as a result`() {
val real = stagedFile("real.mp4", bytes = 4096)
finishedWithOutput(real)
finishedWithNoOutput()
val snapshots = snapshots()
assertEquals("both jobs carry the tag, so both come back", 2, snapshots.size)
val silent = snapshots.single { it.outputPath == null }
assertFalse("no path means no output, not an empty one", silent.outputExists)
assertEquals("and no time either, for the same reason", 0L, silent.outputModifiedAt)
assertEquals(
"the reattachment has to walk past it to the job that really produced a file",
real.absolutePath,
Reattachment.choose(snapshots)?.job?.outputPath,
)
}
private fun snapshots(): List<JobSnapshot> = runBlocking {
workManager.jobSnapshots(
tag = ConversionWorker::class.java.name,
@@ -152,6 +186,11 @@ class JobSnapshotsTest {
).result.get()
}
/** A job that carries the tag and no result key -- still running, or finished without one. */
private fun finishedWithNoOutput() {
workManager.enqueue(OneTimeWorkRequestBuilder<ConversionWorker>().build()).result.get()
}
private companion object {
/** Two fixed moments a day apart, so the ordering is stated rather than raced for. */
const val OLDER_MS = 1_700_000_000_000L
@@ -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")
}
}
@@ -0,0 +1,276 @@
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.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.OutputPublisher
import org.libremediaconverter.convert.SoftwareTranscoder
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.ContainerCapabilities
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.DeviceCodecs
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.Validation
import org.libremediaconverter.model.VideoCodec
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.UUID
/**
* Jobs the worker refuses before it converts anything, and what it says about them.
*
* Two exits, both cold before this file, and both reachable for the same underlying reason: **a job
* does not have to come from the picker.** WorkManager keeps queued and finished work for about a
* week, so a downgrade or a rollback hands this build a job enqueued by another one — the premise
* `WorkerEnumFallbackTest` and `JobTags` are both written on — and `ConversionWorker.request(...)`
* is callable directly.
*
* What makes these worth their own file rather than another case in an existing one is that both
* are about the *message*. A refusal that fails with empty output `Data` renders the UI's generic
* "Conversion failed." with nothing else to say, which is the defect shape `DeniedForegroundStartTest`
* records from the device pass. Asserting the verdict alone would pass against exactly that.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class RefusedJobTest {
private lateinit var app: Application
private lateinit var publisher: OutputPublisher
private lateinit var engine: RefusingTranscoder
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
publisher = AlwaysRoomPublisher(app)
engine = RefusingTranscoder()
ConversionDependencies.publisher = { publisher }
ConversionDependencies.software = { engine }
// Neither test is about probing or about this machine's codecs; both would otherwise decide
// the outcome for reasons no assertion mentions. See WorkerCancellationTest's setUp.
ConversionDependencies.probe = { _, _ -> InputProbe() }
ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }
installTestWorkManager(app, Data.EMPTY)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `a job with no input URI fails with a message rather than a bare failure`() {
val result = runBlocking { workerWithout(ConversionWorker.KEY_INPUT_URI).doWork() }
// `Failure.equals` compares output data, so this pins the message and the verdict together.
assertEquals(
ListenableWorker.Result.failure(workDataOf(ConversionWorker.KEY_ERROR to "No input file.")),
result,
)
}
@Test
fun `a job with no input URI stages nothing`() {
// The URI read is the first thing doWork does -- above the space check, above the staging
// name, above the try. A refusal there must not have reserved anything.
runBlocking { workerWithout(ConversionWorker.KEY_INPUT_URI).doWork() }
assertEquals("a job refused for having no input must not stage a file", emptyList<String>(), stagedNames())
}
@Test
fun `a spec the picker would never have allowed is refused with the reason`() {
// WAV carries PCM and nothing else. The picker cannot produce this combination today, which
// is exactly why the worker checks: the job can arrive from a queue written before the
// settings changed, or from a direct request(...) call.
val expected = ContainerCapabilities.validate(REFUSED_SPEC, InputProbe()) as? Validation.Invalid
?: throw AssertionError("the fixture spec is supposed to be invalid; ContainerCapabilities disagrees")
val result = runBlocking { worker(REFUSED_SPEC).doWork() }
assertEquals(
ListenableWorker.Result.failure(workDataOf(ConversionWorker.KEY_ERROR to expected.message)),
result,
)
}
@Test
fun `a refused spec never reaches an engine`() {
// The half that says it failed *before* converting rather than during. Without this, a
// worker that ran the job and then reported the validation message would pass the test
// above -- and would have spent the user's battery on a file it was going to refuse.
runBlocking { worker(REFUSED_SPEC).doWork() }
assertTrue("a refused spec must be refused before any engine runs", engine.invocations.isEmpty())
}
@Test
fun `a valid spec is not refused`() {
// The control. Every assertion above is about a refusal, so without this they would all
// still pass against a worker that refused everything.
val result = runBlocking { worker(OutputFormat.MP4_H265.spec).doWork() }
assertEquals(ListenableWorker.Result.success(), stripOutput(result))
assertEquals(listOf(OutputFormat.MP4_H265.spec), engine.invocations)
}
// --- the same refusal, on the join side ----------------------------------
@Test
fun `a join of a single file is refused with a message rather than joined`() {
// The arm beside it -- a job with no URI array at all -- is covered on the device by
// `UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage`. This one was covered by
// nothing in either source set, which a coverage report cannot say because it cannot see
// androidTest: the two arms are adjacent lines and only one of them had a test.
//
// Reachable for the reason this file's header gives, plus one of its own: `request(...)`
// takes a `List<Uri>` and checks nothing about its length, so a single-item join is a
// well-formed call, not a corrupted queue entry.
val result = runBlocking { joinWorker(INPUT).doWork() }
assertEquals(
ListenableWorker.Result.failure(
workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.TOO_FEW_INPUTS_MESSAGE),
),
result,
)
}
@Test
fun `a join of two files is not refused for its count`() {
// The control, and the half that makes the test above bite on the boundary rather than on
// the message: without it, `uris.size < 3` passes everything here.
//
// It refuses the space instead of letting the job run, because the next thing past the
// count guard is `ConcatEngine`, which is native -- `NamingPublisher`'s KDoc records that
// no JVM test gets past it. A refusal with the *space* message is proof that execution
// reached line 57, which is proof it got past line 42, and it costs no engine to say so.
val noRoom = NamingPublisher(app).apply { refuseSpace = true }
ConversionDependencies.publisher = { noRoom }
val result = runBlocking { joinWorker(INPUT, SECOND_INPUT).doWork() }
assertEquals(
ListenableWorker.Result.failure(
workDataOf(ConcatWorker.KEY_ERROR to "Not enough free space to join these files."),
),
result,
)
}
/** [ListenableWorker.Result.Success] compares its output data, which these tests do not pin. */
private fun stripOutput(result: ListenableWorker.Result): ListenableWorker.Result =
if (result is ListenableWorker.Result.Success) ListenableWorker.Result.success() else result
private fun worker(spec: OutputSpec): ConversionWorker = build(
workDataOf(
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME,
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
ConversionWorker.KEY_CONTAINER to spec.container.name,
ConversionWorker.KEY_VIDEO_CODEC to spec.videoCodec.name,
ConversionWorker.KEY_AUDIO_CODEC to spec.audioCodec.name,
ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name,
),
)
/**
* The ordinary input `Data`, less one key.
*
* Built by removal rather than by spelling out a shorter map, so the test cannot drift into
* omitting something else as well and passing for a reason it does not name.
*/
private fun workerWithout(key: String): ConversionWorker {
val full = OutputFormat.MP4_H265.spec
val entries = mapOf(
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME,
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
ConversionWorker.KEY_CONTAINER to full.container.name,
ConversionWorker.KEY_VIDEO_CODEC to full.videoCodec.name,
ConversionWorker.KEY_AUDIO_CODEC to full.audioCodec.name,
ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name,
) - key
return build(Data.Builder().putAll(entries).build())
}
private fun build(data: Data): ConversionWorker =
TestListenableWorkerBuilder<ConversionWorker>(context = app, inputData = data, runAttemptCount = 0)
.setId(JOB_ID)
.build()
/**
* A join job carrying [inputs], a declared total, and a format.
*
* The total is declared so `hasRoomFor` takes its `hasSpaceFor` branch: the other branch is
* `hasSpaceForUnknownSize`, which `NamingPublisher` does not override and which would measure
* this machine's real disk.
*/
private fun joinWorker(vararg inputs: Uri): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(
ConcatWorker.KEY_INPUT_URIS to inputs.map(Uri::toString).toTypedArray(),
ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES * inputs.size,
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
),
runAttemptCount = 0,
).setId(JOB_ID).build()
private fun stagedNames(): List<String> =
publisher.createStagingFile("anything").parentFile?.listFiles().orEmpty().map { it.name }.sorted()
private companion object {
val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4")
const val DISPLAY_NAME = "holiday.mp4"
const val INPUT_BYTES = 1024L
/** A join needs two, and "two" is the boundary the count guard is about. */
val SECOND_INPUT: Uri = Uri.parse("file:///tmp/holiday-2.mp4")
/** WAV carries PCM and nothing else, so AAC in WAV has nowhere to go. */
val REFUSED_SPEC = OutputSpec(
org.libremediaconverter.model.Container.WAV,
VideoCodec.NONE,
AudioCodec.AAC,
)
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000005")
}
}
/** An engine that records what it was asked for and writes an output, so a success is a success. */
private class RefusingTranscoder : SoftwareTranscoder {
/** Every spec that actually reached an engine. Empty is the assertion for a refused job. */
val invocations = mutableListOf<OutputSpec>()
override suspend fun run(
request: ConversionRequest,
inputPath: String,
output: File,
durationMs: Long,
onProgress: (Int) -> Unit,
) {
invocations += request.spec
output.writeBytes(ByteArray(OUTPUT_BYTES))
}
private companion object {
const val OUTPUT_BYTES = 512
}
}
@@ -1,12 +1,16 @@
package org.libremediaconverter.work
import android.app.Application
import android.content.Context
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.ForegroundInfo
import androidx.work.ForegroundUpdater
import androidx.work.ListenableWorker
import androidx.work.testing.TestListenableWorkerBuilder
import androidx.work.workDataOf
import com.google.common.util.concurrent.ListenableFuture
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.runBlocking
import org.junit.After
@@ -18,6 +22,7 @@ import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.OutputPublisher
import org.libremediaconverter.convert.SoftwareTranscoder
import org.libremediaconverter.convert.StagingNames
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.DeviceCodecs
@@ -108,6 +113,54 @@ class WorkerCancellationTest {
assertEquals("a failed attempt must not leave its partial behind", emptyList<String>(), stagedNames())
}
@Test
fun `a cancelled join propagates instead of being turned into a Result`() {
val thrown = runCatching { runBlocking { concatWorker().doWork() } }.exceptionOrNull()
assertTrue(
"cancellation must leave doWork as cancellation, not as a Result; got $thrown",
thrown is CancellationException,
)
}
@Test
fun `a cancelled join still deletes the partial it had already staged`() {
// Written first, so a missing delete cannot pass by asking whether a file nobody wrote is
// absent -- the same reason PartialThenFailingTranscoder writes before it throws.
concatStagedFile().writeBytes(ByteArray(PARTIAL_STAGED_BYTES))
runCatching { runBlocking { concatWorker().doWork() } }
assertEquals("a cancelled join must not leave its partial behind", emptyList<String>(), stagedNames())
}
/**
* A join whose foreground start is cancelled rather than denied.
*
* The conversion twin cancels *inside the engine*, which is the honest shape there because
* `ConversionDependencies` has a seam for it. `ConcatWorker` calls `ConcatEngine` directly and
* has no such seam -- it is native, and nothing here gets past it -- so the cancellation is
* injected at the only other point inside the `try`: `setForeground`. That is not a contrivance.
* A job cancelled while WorkManager is promoting it to the foreground is precisely when the
* window is open, and what is being tested is the `catch` arm, which cannot tell where in the
* `try` the cancellation came from.
*/
private fun concatWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(
ConcatWorker.KEY_INPUT_URIS to arrayOf(INPUT.toString(), "file:///tmp/second.mp4"),
ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES,
ConcatWorker.KEY_FORMAT to CONCAT_FORMAT.name,
),
runAttemptCount = 0,
).setId(CONCAT_ID)
.setForegroundUpdater(CancellingForegroundUpdater)
.build()
/** The staging path the join will compute, asked for rather than spelled out here. */
private fun concatStagedFile(): File =
publisher.createStagingFile(StagingNames.forJob(CONCAT_ID, CONCAT_FORMAT.extension))
/**
* A worker routed to the software engine, which is [failure] and nothing else.
*
@@ -142,7 +195,10 @@ class WorkerCancellationTest {
const val DISPLAY_NAME = "holiday.mp4"
const val INPUT_BYTES = 1024L
val SPEC = OutputFormat.MP4_H265.spec
val CONCAT_FORMAT = OutputFormat.MP4_H264
const val PARTIAL_STAGED_BYTES = 2048
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000003")
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000004")
}
}
@@ -167,3 +223,19 @@ private class PartialThenFailingTranscoder(private val failure: () -> Nothing) :
const val PARTIAL_BYTES = 2048
}
}
/**
* Stands in for a job cancelled while WorkManager is promoting it to the foreground.
*
* The mechanism `DeniedForegroundStartTest` documents, carrying a different exception:
* `WorkForegroundUpdater` propagates whatever the future failed with, and
* `ListenableFuture.await()` unwraps the `ExecutionException`, so the worker meets a bare
* `CancellationException` exactly where a real cancellation would put one.
*/
private object CancellingForegroundUpdater : ForegroundUpdater {
override fun setForegroundAsync(
context: Context,
id: UUID,
foregroundInfo: ForegroundInfo,
): ListenableFuture<Void> = FailedFuture(CancellationException("cancelled while going foreground"))
}
@@ -22,6 +22,7 @@ import org.libremediaconverter.model.DeviceCodecs
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.QualityTier
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
@@ -109,6 +110,50 @@ class WorkerEnumFallbackTest {
)
}
@Test
fun `a container this build does not define falls back to the default spec`() {
assertFallsBackToDefault(container = "HOLOTAPE")
}
@Test
fun `a video codec this build does not define falls back to the default spec`() {
assertFallsBackToDefault(video = "H267")
}
@Test
fun `an audio codec this build does not define falls back to the default spec`() {
assertFallsBackToDefault(audio = "SUPER_AAC")
}
/**
* Drives a job whose spec is [NOT_THE_FALLBACK] on every axis but the one named, and asserts the
* whole spec came back as [DEFAULT_SPEC].
*
* **The baseline is the point.** `readSpec` returns the *entire* fallback spec the moment any
* one axis fails to resolve, so a test starting from `MP4_H265` -- which is itself the fallback
* -- could not tell a worker that read the spec correctly from one that gave up on it. Starting
* from MKV/H.264 makes the difference visible on two axes at once.
*
* Asserting the spec that *ran*, rather than only that a `Result` came back, is the other half:
* the defect these three are written for threw out of `doWork` entirely, so "a Result at all"
* would pass against a fallback to something arbitrary.
*/
private fun assertFallsBackToDefault(
container: String = NOT_THE_FALLBACK.container.name,
video: String = NOT_THE_FALLBACK.videoCodec.name,
audio: String = NOT_THE_FALLBACK.audioCodec.name,
) {
val transcoder = RequestRecordingTranscoder()
ConversionDependencies.software = { transcoder }
val result = runBlocking {
conversionWorker(container = container, video = video, audio = audio).doWork()
}
assertEquals(ListenableWorker.Result.success(), stripOutput(result))
assertEquals(listOf(DEFAULT_SPEC), transcoder.specs)
}
/** [ListenableWorker.Result.Success] compares its output data, which these tests do not pin. */
private fun stripOutput(result: ListenableWorker.Result): ListenableWorker.Result =
if (result is ListenableWorker.Result.Success) ListenableWorker.Result.success() else result
@@ -116,15 +161,18 @@ class WorkerEnumFallbackTest {
private fun conversionWorker(
quality: String = QualityTier.FAST.name,
preference: String = EnginePreference.FORCE_SOFTWARE.name,
container: String = SPEC.container.name,
video: String = SPEC.videoCodec.name,
audio: String = SPEC.audioCodec.name,
): ConversionWorker = TestListenableWorkerBuilder<ConversionWorker>(
context = app,
inputData = workDataOf(
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME,
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
ConversionWorker.KEY_CONTAINER to SPEC.container.name,
ConversionWorker.KEY_VIDEO_CODEC to SPEC.videoCodec.name,
ConversionWorker.KEY_AUDIO_CODEC to SPEC.audioCodec.name,
ConversionWorker.KEY_CONTAINER to container,
ConversionWorker.KEY_VIDEO_CODEC to video,
ConversionWorker.KEY_AUDIO_CODEC to audio,
ConversionWorker.KEY_QUALITY to quality,
ConversionWorker.KEY_ENGINE_PREFERENCE to preference,
),
@@ -146,6 +194,12 @@ class WorkerEnumFallbackTest {
const val DISPLAY_NAME = "holiday.mp4"
const val INPUT_BYTES = 1024L
val SPEC = OutputFormat.MP4_H265.spec
/** What `readSpec` returns when any axis fails to resolve. */
val DEFAULT_SPEC = OutputFormat.MP4_H265.spec
/** A spec that differs from [DEFAULT_SPEC] on container *and* video codec. See the helper. */
val NOT_THE_FALLBACK = OutputFormat.MKV_H264.spec
val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000021")
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000022")
}
@@ -156,6 +210,9 @@ private class RequestRecordingTranscoder : SoftwareTranscoder {
val qualities = mutableListOf<QualityTier>()
/** The spec each run was asked for. Which one ran is what the three readSpec tests assert. */
val specs = mutableListOf<OutputSpec>()
override suspend fun run(
request: ConversionRequest,
inputPath: String,
@@ -164,6 +221,7 @@ private class RequestRecordingTranscoder : SoftwareTranscoder {
onProgress: (Int) -> Unit,
) {
qualities += request.quality
specs += request.spec
output.writeBytes(ByteArray(OUTPUT_BYTES))
}
@@ -1,10 +1,14 @@
package org.libremediaconverter.work
import android.content.Context
import com.google.common.util.concurrent.ListenableFuture
import org.libremediaconverter.convert.OutputPublisher
import org.libremediaconverter.convert.SoftwareTranscoder
import org.libremediaconverter.model.ConversionRequest
import java.io.File
import java.util.concurrent.ExecutionException
import java.util.concurrent.Executor
import java.util.concurrent.TimeUnit
/**
* Scaffolding more than one worker test needs.
@@ -68,3 +72,25 @@ object WritingTranscoder : SoftwareTranscoder {
private const val OUTPUT_BYTES = 512
}
/**
* An already-failed future, written out rather than pulled from a futures library.
*
* `await()` takes the `isDone` fast path and unwraps the `ExecutionException`, which is what puts
* the original exception in front of the worker's `catch` rather than a wrapper. That is the whole
* mechanism behind driving a `ForegroundUpdater` to fail: `WorkForegroundUpdater` propagates
* whatever the future failed with rather than swallowing it, so `setForeground()` throws exactly
* what is handed here.
*
* Shared because two tests inject two different failures through it -- a denied foreground start
* and a cancellation -- and Kotlin will not take two file-private top-level classes of one name in
* one package.
*/
internal class FailedFuture(private val failure: Throwable) : ListenableFuture<Void> {
override fun addListener(listener: Runnable, executor: Executor): Unit = executor.execute(listener)
override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false
override fun isCancelled(): Boolean = false
override fun isDone(): Boolean = true
override fun get(): Void = throw ExecutionException(failure)
override fun get(timeout: Long, unit: TimeUnit): Void = throw ExecutionException(failure)
}