Two results from #223-#230 that belong with the read rather than only in
their own tickets.
E7 re-scoped its own ticket. #226 split into a cheap headless half and an
expensive picker-driven one, on the premise that a real DocumentsProvider
can be reached without DocumentsUI. It cannot: an unprotected one is
refused at install, instrumentation runs in the app's uid so the test APK's
own identity is no help, and adopting shell identity is denied too -- each
denial naming ACTION_OPEN_DOCUMENT as the only way in. Measured three ways.
So #226 is one item at the picker's cost, not two.
The useful half of that distinction is that the input bridge needs no
documents provider at all. getSafParameterForRead opens a descriptor
through the resolver, so any readable content:// URI exercises it, which is
what kept #225 headless.
And that is how the read's one production defect surfaced. #238: joining
files picked through the system picker failed outright on the stream-copy
path, because the concat demuxer whitelists protocols separately from
-safe 0 and ffkitsaf was not on the list. Only STREAM_COPY feeds the
demuxer a list file, and every existing join test passed Uri.fromFile, so
the one broken combination was the only one a user could reach.
Worth stating plainly next to the coverage entry: it was not a missed line
and not an unasserted value, but two covered things no test put together --
the gap shape a coverage number is worst at, and the reason the read
happened.
E4 is marked fixed; #243 made that KDoc name the constant rather than
restate it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two FFmpeg engines were done in ad2a75d and d293646. This is
Media3Engine.transcode's invokeOnCancellation, which posts
transformer.cancel() onto the engine's own HandlerThread because cancel()
has the same single-thread requirement as start().
The assertion is the output file here, where it could not be for FFmpeg.
That side deletes the partial on cancellation, and on POSIX ffmpeg keeps
writing to the unlinked inode, so the path stays gone whether or not the
cancel landed -- it asserts the session's return code instead. Media3Engine
deletes nothing, the partial being ConversionWorker's to clean up, so the
file is the evidence.
A cancelled export reports itself two ways and both mean interrupted: no
video track, or MediaExtractor refusing the file outright with "Failed to
instantiate extractor" because there is no moov atom. The first version
treated only the null as success and the exception failed the test, which
is how that was measured. Only a playable file counts as a miss.
The wait before reading is several times the export's own length, so a
cancel that did not land has certainly finished by then: the failure
direction is "the file became playable", never "we did not wait long
enough". The attempt is retried for the reason the other two engines
measured -- a 3 s 320x240 export outruns a naive cancel on a loaded runner
-- and an export that never wrote a file at all is recorded as
inconclusive rather than allowed to pass as a cancellation.
It carries @FailsOnEmulatorApi37, so FAILS_ON_EMULATOR_API37_BASELINE moves
3 -> 4 in this diff. That file also said removing the marker would grow the
gating leg "by two", which has been wrong since the third marker landed; it
now names the constant instead of restating it.
Verified on a local API 34 emulator: 68 tests, 0 failures, 3 skipped; and
with transformer.cancel() removed all five attempts produce a playable
video/hevc and the test fails, naming each one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reattachment.rank gives RUNNING the highest rank of all -- "live work
outranks a finished result because a running job is holding a foreground
service" -- and no test on either source set had ever produced one.
ReattachOnLaunchTest covers a job that finished, one whose staged file is
gone, an ambiguous pair, one still queued, and one the user cancelled.
ReattachmentTest exercises the ranking as a pure function over fabricated
snapshots. What was missing is a ViewModel meeting a real running job,
which is also the likeliest reattachment there is: the user starts a
conversion, leaves, and comes back while it is still going.
The engine is a fake, deliberately. The job has to still be running when
the ViewModel is built, and every real conversion in this suite finishes in
about a second -- racing that is what made the cancellation tests flaky
enough to need retries. A SoftwareTranscoder that blocks until released
removes the race outright. Nothing about reattachment depends on which
engine is transcoding: the tag query, Reattachment.choose over live
WorkManager state, and observe's mapping to Converting all run identically
whatever is doing the work.
This is what #230 can actually deliver, and the ticket asked for the answer
either way. Process death itself stays device-manual. D3/D13 already record
that am kill refuses a process holding a foreground service, and there is a
more basic obstacle underneath it: instrumentation runs in the app's own
process, so any route that really killed it would take the test runner with
it and leave nothing to assert with. Observing a relaunch needs two
instrumentation runs, which the runner does not provide. So the closest
observable analogue is a fresh ViewModel, with no memory of the work,
meeting a job that is genuinely mid-flight.
The teardown now resets ConversionDependencies. The suite runs without
Android Test Orchestrator, so a BlockingTranscoder left in place would hang
the next class that converts anything.
Verified on a local API 34 emulator: 67 tests, 0 failures, 3 skipped; and
making RUNNING unreattachable in Reattachment.rank fails this test and
nothing else -- which is also the evidence that the JVM ranking test was
not already covering it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Joining files picked through the system picker failed outright whenever
the strategy was stream copy -- the matched-files case the UI advertises
as "joined without re-encoding, no quality loss".
[ffkitsaf @ ...] Protocol 'ffkitsaf' not on whitelist 'file,crypto,data'!
Error opening input file .../joined_from_content.concat_list.txt
JoinScreen picks with OpenMultipleDocuments, so real inputs are always
content://. ConcatEngine maps each through getSafParameterForRead and
FFmpegConcatCommand writes the resulting ffkitsaf: paths into the concat
list file. The demuxer applies its own protocol whitelist, defaulting to
file,crypto,data, and -safe 0 does not touch it: that permits absolute
paths, this permits the scheme they carry. Two separate gates, and only
one was open.
Nothing caught it because the two halves of the bug never met. Only
STREAM_COPY feeds the demuxer a list file -- REENCODE passes each input
with its own -i, where the whitelist does not apply -- so joining over SAF
worked for mismatched clips. And every join test passed Uri.fromFile,
which takes ConcatEngine's uri.path arm instead of the bridge, so
matchingClipsAreJoinedByStreamCopy exercised stream copy with a file:
path and passed. The one broken combination was the one no test produced
and the only one a user can reach.
That is #225's gap: FFmpegKitConfig.getSafParameterForRead is on every
real conversion and join, and was on no passing test -- only on
UnopenableUriTest's failure side, which proves the error message rather
than the bridge. ContentUriInputTest now drives both the convert and join
paths from a real content:// URI.
It uses a plain ContentProvider, because the documents provider cannot be
reached. Measured three ways: a DOCUMENTS_PROVIDER without MANAGE_DOCUMENTS
is refused at install, instrumentation runs in the target app's process so
Instrumentation.getContext() still carries the app's uid and is denied, and
adoptShellPermissionIdentity(MANAGE_DOCUMENTS) is denied identically -- the
denial naming ACTION_OPEN_DOCUMENT as the only way in. The bridge needs no
documents provider: it opens a descriptor through the resolver, so any
readable content:// URI exercises it, and an ordinary provider may be
exported unprotected. The whole class stays headless. Recorded on #226,
which that also settles: its cheap half does not exist.
Verified on a local API 34 emulator: 66 tests, 0 failures, 3 skipped; and
removing the -protocol_whitelist pair reproduces the production failure
verbatim in the join test and nothing else. FFmpegConcatCommandTest pins
the flag on the JVM.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
onInputPicked does not reach Ready on the calling thread. It hops twice --
withContext(pickDispatcher) { InputQuery.describe(...) } and then the probe
-- and pickDispatcher defaults to Dispatchers.IO, a real background thread
Compose's idling knows nothing about. So deliver() returned with the state
still Idle, and asserting immediately was a race the test usually won.
It lost five times on CI in one day, on PRs whose diffs were instrumented
tests and documentation and could not reach it. Two of those failures came
alongside #125's deadlock and could be argued as fallout; three did not.
waitUntil polls through waitForIdle, draining the main looper each time, so
it sees the recomposition the IO hop eventually posts back.
Injecting the dispatcher would be better and is not available here.
pickDispatcher is a constructor parameter precisely so a test can pin it,
but this test composes the real ConverterScreen, which resolves its own
ViewModel through viewModel() -- the seam is one layer below the launcher
edge this class exists to cover, and reaching for it would mean not testing
that edge.
The evidence is the mutation rather than the repetition count, per the note
#218 left: transposing the two launcher callbacks at ConverterScreen.kt:70
and :83 -- the exact defect this test guards -- still fails it, so the wait
did not make it vacuous. A transposed callback leaves the screen in Idle
forever and it fails on the timeout with the meaning it had before.
Supporting evidence, eight consecutive green runs of the class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both cancellation tests I added in ad2a75d and d293646 wait for
SessionState.RUNNING and then cancel. That is not enough. The conversion
one passed four consecutive local runs and all five CI legs, then failed
the API 34 and 35 legs of the next PR with state=COMPLETED rc=0, on a diff
that could not reach it. On a loaded runner the thread that observed
RUNNING can be descheduled long enough for a short encode to finish before
it calls cancel. A longer timeout does not help: the wait already
succeeded.
Two changes, because neither is sufficient alone.
A slower encode. The conversion test now targets WEBM_VP9 at BEST, the
slowest thing FFmpegCommandBuilder emits -- libvpx-vp9 -crf 31 -b:v 0,
with -deadline realtime added only on FAST. Probed on an API 34 emulator:
that session is still RUNNING at 1 s and finished by 2 s, against well
under a second for x265 -preset medium.
A bounded retry. An attempt whose session finished before the cancel
landed has not tested anything, so it is a miss rather than a failure and
is retried; only exhausting five attempts fails, and the message reports
every attempt's state and return code so a real breakage is distinguishable
from a slow machine.
The retry does not soften the test. With FFmpegKit.cancel removed from
both engines, every attempt ends COMPLETED, so both still fail -- verified,
each listing five [state=COMPLETED rc=0] outcomes. Two clean runs
beforehand at 64/0/0/3.
The join test gets the same treatment. It has not flaked yet, but it is
the same mechanism and the same fragility, and finding out on CI again is
not worth the round trip.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The FFmpegEngine half landed in ad2a75d; this is the same gap in
ConcatEngine. Between them, a real native session being asked to stop is
now covered on both FFmpeg paths.
The assertion is the session's return code again, and here that is not
merely the better choice but close to the only one: ConcatEngine does not
delete its output on cancellation at all. Its invokeOnCancellation is
FFmpegKit.cancel and nothing else, where FFmpegEngine's also deletes the
partial. Whether that asymmetry is deliberate is a separate question, so
this asserts what is true of both engines rather than depending on it.
The cancel triggers on SessionState.RUNNING rather than on progress.
ConcatWorker publishes no progress at all, so there is no callback to hang
it on even in principle -- and the conversion side already measured the
deeper reason, that the committed clips outrun a callback-triggered cancel.
The inputs are the mismatched pair on purpose, so ConcatPlanner chooses
REENCODE. A stream copy of two 2 s clips is close to instantaneous and
would leave nothing to interrupt; re-encoding is also the case where a user
would actually reach for Cancel.
Verified on a local API 34 emulator: three runs green at 64/0/0/3, and
replacing invokeOnCancellation's body with an empty block fails this test
and nothing else, with state=COMPLETED rc=0.
Media3Engine's transformer.cancel() is still uncovered and #224 stays open
for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every cancel in app/src/androidTest is WorkManager.cancelWorkById against
work that is queued or already finished: ReattachOnLaunchTest cancels a job
carrying a one-hour initial delay, and another immediately after enqueue.
On the JVM, WorkerCancellationTest and HardwareFallbackTest's cancellation
case drive a SoftwareTranscoder double that records the call. No test on
any source set had asked a real native session to stop. That is
docs/defect-audit.md D10's forcing condition.
It is the one path where cancelling wrong is silently expensive rather than
loudly broken: a missed FFmpegKit.cancel leaves the native process encoding
to completion while the UI says the job is cancelled.
Two things were measured rather than assumed, and both changed the test.
The output file cannot be the assertion. invokeOnCancellation deletes the
path, and on POSIX unlinking a file ffmpeg still holds open leaves ffmpeg
writing to the unlinked inode -- so the path stays gone whether or not the
cancel reached the session, and removing FFmpegKit.cancel passes that check
every time. The session's own verdict is what separates them: a cancelled
session ends with the cancel return code, a completed one does not.
Cancelling from the first progress callback loses the race. It was tried
first and failed with state=COMPLETED rc=0: every committed fixture is
2-3 s at 320x240, and the encode finishes before the first statistics
callback is delivered and acted on. FFmpegKit.listSessions shows the
session RUNNING far earlier, so that is what the test waits for.
QualityTier.BEST is deliberate for the same reason -- preset medium leaves
more of the encode ahead of the cancel.
Verified on a local API 34 emulator. Four consecutive runs green at
62/0/0/3, and removing FFmpegKit.cancel while keeping output.delete fails
this test and nothing else, with state=COMPLETED rc=1.
ConcatEngine and Media3Engine carry the same shape and are not covered
here; #224 stays open for them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ConversionNotifications.build attaches one action, wired to
WorkManager.createCancelPendingIntent(id). Until now
createCancelPendingIntent had no references anywhere outside its own
declaration -- no JVM test, no instrumented test.
That is worth more than an ordinary uncovered line. A conversion runs in a
foreground service and the user is invited to leave the app; once they do,
this action is the only way to stop it. If the PendingIntent carries the
wrong id the button does nothing, the notification stays, and the job runs
to completion, with no error, no log and no screen to look at.
The obvious version of this test reads NotificationManager's active
notifications for id 1001 and taps what it finds. Rejected: the
instrumented suite grants no runtime permissions, so POST_NOTIFICATIONS is
denied throughout, and whether a suppressed foreground-service notification
is returned there is a platform detail that varies. The test would be
asserting something about notification visibility rather than about
cancellation. The PendingIntent is the subject and where it is read from is
incidental, so this builds the notification for a real live work id and
fires its action -- a real dispatch reaching real WorkManager, the same way
on every API level.
The job carries an initial delay so it stays ENQUEUED. A conversion of the
3 s fixture finishes in well under a second on an emulator, so racing a
cancel against a running job would be flaky in the direction that fails,
and cancelWorkById acts on ENQUEUED identically. What is under test is
whether firing the action reaches WorkManager with the right id.
Verified both ways on a local API 34 emulator: 61 tests, 0 failures, 3
skipped as written; and building the PendingIntent from a random UUID
instead of the request's id fails the new test and nothing else.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FFmpegEngine derives progress as stats.time / durationMs * 100, and the
statistics callback runs on every conversion in FFmpegEngineTest -- they
all pass durationMs = 3_000. But every call site omits onProgress, so
nothing on any source set had ever looked at the number. Replacing percent
with a constant reddened nothing.
What already existed covers the plumbing downstream and not this: #196
covered the worker's progress lambda with a fake engine that reports
whatever the test tells it to, and ProgressNotificationTest covers the
throttling the same way. The arithmetic was the one part with no reader.
The new test passes 30 s as the duration for a fixture that is exactly
3.000 s, so the conversion still encodes the whole clip and the reported
percentage tops out around 10 rather than 100.
That is what makes it bite. A range check alone is worthless: a constant 0
satisfies both "every value is in 0..100" and "the values never go
backwards", and so does a list of [0, 100]. Pinning the band rejects every
constant, and because the band sits a tenth of the way up it also rejects
an implementation that ignores durationMs, which would report ~100 for the
same run. The bound is loose -- 5..25 for an expected 10 -- because the
last statistics callback can land slightly before the final frame.
Verified on a local API 34 emulator: 61 tests, 0 failures, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
HardwareFallbackTest is the only automated check of the hardware->software
fallback against a real codec failure, and it passed on every CI leg
without ever attempting the hardware path.
Measured on run 34004304566: the API 33, 34, 35 and 37 legs each log
Routing sample_h264_444.mp4 -> ... via FFMPEG (NO_HARDWARE_ENCODER)
Emulators expose no hardware encoder, so the router never chooses Media3
and runMedia3OrFallBack's catch is never entered. The test's assertions --
succeeded, output non-empty -- are true of that conversion too. It
finished in 448 ms, which is not long enough to fail an export and then
software-encode a three-second clip. Deleting the catch reddened nothing.
The ticket offered two fixes and left the choice open. Trying the first
one answered it, and not the way the ticket expected. Pinning
deviceCodecs to PERMISSIVE, as ForcedFailureTest does, makes the router
choose Media3 -- and the export then SUCCEEDS. On a local API 34
emulator, MediaCodecInfo logs
NoSupport [codec.profileLevel, avc1.F4000C, video/avc]
for both c2.goldfish.h264.decoder and c2.android.avc.decoder, and
ExoPlayer allocates the goldfish decoder anyway, which decodes the High
4:4:4 fixture regardless of the profile it declares. c2.android.hevc.encoder
then encodes it and the job reports MEDIA3.
So the class KDoc's "Media3 fails partway through the export on every
device" is not true of the emulator images, and no routing pressure makes
this fixture force a fallback there. Pinning would also swap in software
codecs, which is not the path a real device takes -- it is what made the
forced run succeed.
That leaves assumeTrue on the production premise as the honest answer, now
with a measurement behind it rather than a coin flip. The test skips where
it cannot mean anything and runs on the Pixel, where it always could.
When it does run the assertion is a pair, because KEY_ENGINE_USED is
FFMPEG whether the fallback fired or the router went straight there:
the router chose MEDIA3 for this request on this device, AND the worker
reported FFMPEG. Together, and only together, that is the fallback.
Verified on a local API 34 emulator: the test reports SKIPPED and the
level reports skipped=3. ForcedFailureTest still covers the fallback
wiring on every leg with a double; what needs a real encoder is two real
engines disagreeing about a real file.
The third permanent skip is recorded in docs/local-emulator.md and beside
SafPickerRoundTripTest's run-shape note.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
encodesFlacLosslessAudio and encodesOpus asserted only that a non-empty
file appeared. Five siblings in the same class check what is in it --
encodesWav reads RIFF two lines away, and GIF, Matroska, MP3, H.264 and
H.265 all assert a container marker or a track MIME.
convert() throws on a non-zero return code, so these two did prove the
command ran. What they could not distinguish is the command running and
producing the wrong thing, which is a failure mode this codebase has
already had once: F1 in docs/coverage-read-findings.md records a live
Vorbis arm in FFmpegCommandBuilder that ContainerCapabilities says cannot
exist. Pointing OutputFormat.FLAC's arm at pcm_s16le left both tests green.
Four bytes each, in the idiom the class already uses. OutputFormat.FLAC is
Container.FLAC, whose ffmpeg format is "flac", so the file opens with the
native stream marker fLaC. OutputFormat.OPUS is Container.OGG -- "ogg" --
so it is an Ogg stream and opens with OggS. Verified against both muxers
directly rather than assumed from the codec name; the marker belongs to the
container, so it does not vary with the ffmpeg build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four coverage waves have been steered by JaCoCo, which measures
testDebugUnitTest only and cannot see app/src/androidTest at all. So
nothing had ever asked what the 60 device tests pin, only that they were
green. This is that read: a triage, not a test push, in the shape of
docs/coverage-read-findings.md.
Six findings (E1-E6) are things a test would not fix, and five of those
six are prose rather than code — the suite itself is in good condition.
Eight tickets carry the rest (#223-#230), each naming the mutation that
has to go red rather than a coverage delta.
The one that matters is #223. HardwareFallbackTest is the only automated
check of the hardware->software fallback against a real codec failure,
and it has never attempted the hardware path. Measured on run
34004304566: the API 33, 34, 35 and 37 legs each log
Routing sample_h264_444.mp4 -> ... via FFMPEG (NO_HARDWARE_ENCODER)
because emulators expose no hardware encoder, so the router sends the job
straight to FFmpeg and runMedia3OrFallBack's catch is never entered. Its
two assertions — succeeded, output non-empty — are true anyway. It
finishes in 448 ms, which is not long enough to fail a hardware export
and then software-encode a three-second clip. Deleting that catch reddens
nothing anywhere.
Two things generalise. A test can assert and still not reach, which
neither a coverage number nor a "does it assert something" review can
see; the filter that works is whether the test's premise holds on the
machine running it. And the codebase already knew — ForcedFailureTest
pins DeviceCodecs.PERMISSIVE against this exact hazard and says why, as
does ConversionWorkerTest. Their assertions are about the path, so
without the pin they fail loudly; HardwareFallbackTest's are about the
output, so it passes quietly. That asymmetry is why nobody noticed.
E5 records the structural reason this document is separate: F7 in the
coverage findings calls probeWithExtractor's catch uncovered when
RemuxTest drives it on a device every leg. A JaCoCo-derived document
cannot see androidTest, so it will keep re-deriving that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Numbers re-measured on 6004398 rather than inherited: 94.2% line (2234/2372),
87.5% branch (1171/1338), 628 tests in 96 classes, missed 138 lines and 167
branches.
The entry explains its own branch move, because this file has a history of not
doing that. Wave 4's rise is almost entirely numerator -- up 80, with the
denominator moving -4 -- unlike the 2026-08-29 seam work, where the numerator
rose 37 while the denominator fell 70 and much of the gain was scaffolding
leaving the measurement. The line denominator grew 20, which is the seams the
wave cut rather than untested code.
Also records a methodological result worth more than the percentages: #218 fixed
an intermittent flake whose six-runs-per-arm comparison caught nothing either way
and proved nothing, and was settled by a deterministic mutation instead -- then
confirmed when the race reproduced on #217's leg, below the fix.
The JUnit-timeout trap gains the outcome it was waiting for: the jstack watchdog
caught #125's Room/WorkManager deadlock on CI in 10m57s with the hung test named,
against that ticket's prediction of a 60-minute cap and no cause. #125 is closed
as bounded; the inversion is internal to the two libraries and still live at
work-runtime 2.11.2 / room 2.7.0.
Instrumented counts are untouched: #221 is what changes them, so it carries them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thePickedInputSurvivesARealRotation synchronised a rotation with waitForIdle(),
which waits for the compose hierarchy to settle. Right after a rotation the
window manager has accepted but not yet delivered as a configuration change, the
old Activity's composition is already idle -- so it returns, composeRule.activity
still resolves to the old instance, and the guard reads an unchanged identity
hash. Nothing waited for MainActivity to be rebuilt.
That is the clean AssertionError on #217's API 33 gating leg, run 33698846104:
it failed the SECOND guard, so the first had passed and the display really had
rotated. The wedges this ticket opened with are the same race taken the other
way -- land while the composition is being torn down and waitForIdle has nothing
coherent to settle on.
awaitRecreation() waits on a counter fed by the runner's lifecycle monitor,
bounded at 15 s. Deliberately not polling composeRule.activity: that resolves
through scenario.onActivity, which blocks on the main thread, so polling it
across a recreation is a plausible reading of the very wedge being fixed.
Both guards stay. The identity-hash one is now a backstop rather than the
primary detector -- a configChanges attribute trips the barrier's timeout first,
with a message saying what was waited for.
Measured on the local API 33 emulator. With the fix, 60/60 green. With the
watcher mutated so the counter never increments, the test fails in 15 s naming
itself and its condition, and the run still reports received 60/60 completed
cleanly -- where the same missing recreation used to cost the leg 20 minutes and
name nothing. The pre-fix flake does not reproduce on this host, so that is a
demonstration of the timeout path, not a before/after.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Robolectric builds an Application per test class that asks for one, and each
onCreate launched a staging sweep on Dispatchers.IO over the shared
<cacheDir>/conversions/. Nothing joined them, so a test asserting about a staged
file was racing every sweep the classes before it had left in flight (#159).
It was CI-only until wave 4 added ten Robolectric classes, at which point
OutputPublisherStagingTest started failing locally too.
LibreMediaConverterApp gains a protected open sweepScope and publishes the Job
onCreate started; the JVM suite substitutes TestLibreMediaConverterApp, whose
scope is Dispatchers.Unconfined so the sweep -- a plain function that never
suspends -- runs to completion inline. The SupervisorJob is kept so this differs
from production in the dispatcher alone.
Suite-wide rather than per-test: 27 of the 58 Robolectric classes touch that
directory, so opt-in was not a real option.
It costs one assertion, knowingly. AppStartSweepTest opened by asserting that
the manifest's android:name is what Robolectric instantiated. An application=
override replaces the manifest rather than being checked against it, and
applicationInfo.className reports the override too, so that claim is now
unobservable from this source set and a rewritten version would assert the
override against itself. The manifest link is device-only; the cast in setUp
still catches the test app ceasing to extend the real one.
AppStartSweepTest also joins the published Job instead of polling for ten
seconds -- a poll cannot tell "swept" from "not started yet" -- and gains a test
pinning that the sweep is complete when onCreate returns, which is the property
the substitution exists for and the only place it is checked.
Verified by mutation rather than by repetition. Putting the test app back on
Dispatchers.IO reddens that test 5 times out of 5, while running the whole suite
six times per arm caught nothing either way: at the rate #159 was observed at, a
clean six-run arm is roughly a coin flip, so the comparison was underpowered and
is not offered as evidence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FFmpegEngine and ConcatEngine each carried their own copy of the same `when`, and the copies
had drifted: one preferred the fail stack trace and fell back to the log tail, the other only
ever read the log tail. Neither was tested -- both live inside a callback handed to FFmpegKit,
which does not run on the JVM -- so nothing could see that the two disagreed about what a
failed session says.
sessionOutcome() now holds the rule and each engine maps Success/Cancelled/Failed onto its
continuation. Verified JVM-safe rather than assumed: javap over the committed AAR shows
ReturnCode(int) as a plain public constructor with pure static isSuccess/isCancel and a
<clinit> that loads no native library.
Per #203's decision this unifies on the stack trace, so a join failure now carries the
diagnostics a conversion failure always did. The PREFIX stays per-engine: unifying the
strategy must not unify the sentence, since a join reporting "FFmpeg failed" would be a worse
message than the one it replaces. There is a test for exactly that.
The two message sources are lambdas rather than values, and that is load-bearing.
getFailStackTrace and getAllLogsAsString are calls onto a native session, and only the
failure arm needs either; taking them by value would put both on the happy path of every
successful conversion, which the shape this replaces did not -- it read them inside the else
branch. Same reasoning as capabilitiesFrom taking a Sequence in #194: a seam should not change
what runs when. There is a test that counts the reads, and the eager mutation reddens it.
A null return code is a real input rather than a defensive one -- getReturnCode() is nullable
and a session killed before reporting has none -- so it fails, with "null" where the number
would be.
Nothing asserted the old join text: `grep -rn 'Joining failed|FFmpeg failed' app/src/` returns
only main, plus ConcatWorker.GENERIC_FAILURE_MESSAGE, which is a different constant this does
not touch. Re-run immediately before committing, as the ticket asked.
Mutations, all run and restored:
swap the ifBlank operands 1 red
treat cancellation as a failure 2 red
read both message sources eagerly 1 red
hardcode the prefix 1 red
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
currentInput() answered for Converting, Waiting and Converted as well as Ready. Those three
arms were unreachable by tapping Convert -- the button renders only in the Ready branch --
but they were reachable through the POST_NOTIFICATIONS *result*, which ConverterScreen.kt:91
wires to convert() rather than to the button.
Reaching one enqueued a SECOND job over a live one: activeWorkId was overwritten, and the
first job kept running with its foreground notification orphaned and nothing left holding
its id to cancel it.
#202 decided to narrow rather than to test it as it stood, because a test written against
the old shape would have frozen the double-enqueue as intended behaviour -- the F1/F5 failure
mode docs/coverage-read-findings.md names. currentInput() is now
(_state.value as? ConversionState.Ready)?.input, which is what JoinViewModel.join() has been
all along; the two screens are the same shape and only one of them was over-general.
Four cold refusal arms come with it, all reached the same way -- a system callback arriving
after the screen has moved on, which is what a result redelivered after process death does:
ConversionViewModel.kt:513 currentInput() ?: return
ConversionViewModel.kt:600 pendingSave() ?: return
JoinViewModel.kt:316 (as? Ready)?.inputs ?: return
JoinViewModel.kt:390 pendingSave() ?: return
One fixture note worth keeping: the second case needs a real staged file in the worker's
output Data. A SUCCEEDED job with no output path maps to Failed rather than Converted, so
Data.EMPTY never reaches the state the case is about -- which cost a timed-out awaitState
before it was spotted.
Mutations, all run and restored:
restore the over-general four-arm when 1 red <- the defect this change fixes
currentInput()!! at :513 2 red
pendingSave()!! in save() 1 red
drop both join guards 1 red
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Jason Ross
JMR-devandClaude Opus 5